dnhn / df-fe-23

https://dffe23-dn-7.vercel.app
1 stars 0 forks source link

Assignment 3 #6

Open dnhn opened 11 months ago

dnhn commented 11 months ago

https://dffe23-dn-3.vercel.app

trankhacvy commented 11 months ago

Hello @dnhn , solid work!

Requirements

Final result: 🔥 Passed

Feedback

We also have some comments for your work:

  1. When we assign a value to our variables, TypeScript can automatically figure out the type from that value. So, you don't have to explicitly declare the type for these kinds of variables.

https://github.com/dnhn/df-fe-23/blob/be2b79039d42fa066088155c00623ca961bce1fe/assignment-3/src/common/constants.ts#L1

  1. There are components and functions that are missing types. If you enable the noImplicitAny setting, you'll be able to spot them. Consider adding types to them.

https://github.com/dnhn/df-fe-23/blob/be2b79039d42fa066088155c00623ca961bce1fe/assignment-3/src/components/books/BooksContext.tsx#L65

https://github.com/dnhn/df-fe-23/blob/be2b79039d42fa066088155c00623ca961bce1fe/assignment-3/src/components/books/TableRow.tsx#L9

  1. We can calculate the filtered books from the bookList so there's no need to maintain a separate state for it. You might want to use the useMemo hook in this situation.

https://github.com/dnhn/df-fe-23/blob/be2b79039d42fa066088155c00623ca961bce1fe/assignment-3/src/components/books/Table.tsx#L16

  1. You don't have to disable this rule; simply include the setPage function in the dependencies array.

https://github.com/dnhn/df-fe-23/blob/be2b79039d42fa066088155c00623ca961bce1fe/assignment-3/src/components/books/Table.tsx#L29

You also deserve praise for including extensive typing, integrating lint-staged, and adding unit tests. Keep up the excellent work! 🔥