dnhn / df-fe-23

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

Assignment 6 #12

Open dnhn opened 11 months ago

dnhn commented 11 months ago

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

ngolapnguyen commented 10 months ago

Great job 👍

Requirements

Result: ✅

Feedbacks

  1. useSWR first params is the dependency array. You shouldn't have to mutate it manually:
image

Instead:

useSWR(['books', page, query, ...], () => getBooks(...))
  1. Very nice idea with using window event to logout 👍

  2. Nice 👍

image
  1. I'd recommend calling setQuery in BooksContext instead of TableToolbar:
image

Same thing for setting initial page value from search params. Because we are managing them with BooksContext, we should set the initial values for them there instead of from inside random components. It'd be very confusing (e.g. how do we know we are setting default value?), hard to find & prone to side effects.

  1. page params is not being set as initial value correctly. Right now when I refresh, it always get reset to 1.

  2. ... (Please refer to my feedbacks for previous assignments)

dnhn commented 10 months ago

Hello @ngolapnguyen, thanks for reviewing. I have some responses for your feedback:

Same thing for setting initial page value from search params. Because we are managing them with BooksContext, we should set the initial values for them there instead of from inside random components. It'd be very confusing (e.g. how do we know we are setting default value?), hard to find & prone to side effects.

  1. page params is not being set as initial value correctly. Right now when I refresh, it always get reset to 1.

I once stored these parameters in the context but they did not work as expected. Updating a parameter even reset the other. Therefore I decided to only retrieve the q due to its importance compared to the page parameter. I have yet to figure out how to handle this properly, I saw you mentioned using query-string in the previous assignment, let me try.


SSR with cookies

Could you please show me how to implement this properly?

ngolapnguyen commented 10 months ago

@dnhn You should be able to get cookies from server side with cookies. A simple approach for SSR would be saving access token in browser cookie, and use it for data fetching.

This way you should be able to get books & stuff on the server side & achieve SSR.