Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use useMemo for hydration and avoid unnecessary hydrates #510

Merged
merged 4 commits into from
Dec 28, 2022

Conversation

kirill-konshin
Copy link
Owner

@kirill-konshin kirill-konshin commented Dec 15, 2022

  • Upgrade packages and refactor a bit
  • Added necessary testing dependency
  • Stop hydrating on server and use useLayoutEffect for client hydration
  • Added pokemon page with rtk's createApi
  • Added back dispatch in GSP in demo repo
  • A change in query params constitutes a new page now
  • Improve performance by using another hook on server
  • Add detail page
  • New approach: split gsp and gssp and hydrate based on those
  • Added a second type of initial state handling with more explanations
  • Improved useMemo comment
  • Make sure hydrates work when staying on the same page
  • Add links to demo repo to test issue (seems like no issue)

Supersedes #502

Fix #493 #495 #496

* Upgrade packages and refactor a bit
* Added necessary testing dependency
* Stop hydrating on server and use useLayoutEffect for client hydration
* Added pokemon page with rtk's createApi
* Added back dispatch in GSP in demo repo
* A change in query params constitutes a new page now
* Improve performance by using another hook on server
* Add detail page
* New approach: split gsp and gssp and hydrate based on those
* Added a second type of initial state handling with more explanations
* Improved useMemo comment
* Make sure hydrates work when staying on the same page
* Add links to demo repo to test issue (seems like no issue)
@voinik
Copy link
Contributor

voinik commented Dec 15, 2022

I’m done with #512 so feel free to merge it jnto this branch :) What are your thoughts regarding merging this PR to master?

@kirill-konshin
Copy link
Owner Author

I’m done with #512 so feel free to merge it jnto this branch :) What are your thoughts regarding merging this PR to master?

I am going to test everything once again, and release 8.1.0.

@voinik
Copy link
Contributor

voinik commented Dec 15, 2022

Alright sounds good!

Regarding page vs app GIP:
The getInitialAppProps (giap) and getInitialPageProps (gipp) differ slightly. My version of useWrappedStore grabbed the relevant state out of props?.pageProps?.initialState. But it would only do that if there was a props?.__N_SSG or props?.__N_SSP property set by next. However if gipp is used, no such property exists, but the state does exist inside props?.pageProps?.initialState. So my version just ignored the state in that instance. The fix is to check if there’s no gsp and no gssp state, and in that case check if there’s props?.pageProps?.initialState. If there is, that has to be our gipp state.

Can’t write a testcase today, it’s 23:00 and I’m about to sign off for the evening. I can do it tomorrow if you wish!

This was referenced Dec 15, 2022
@kirill-konshin
Copy link
Owner Author

Yes, please. It can wait.

@voinik
Copy link
Contributor

voinik commented Dec 16, 2022

@kirill-konshin Done, check out #514

@voinik
Copy link
Contributor

voinik commented Dec 16, 2022

An issue was reported with the useMemo approach: #502 (comment)

@karaoak mentioned how he fixed that issue in a project (#502 (comment)). You need to do data reconciliation on pages where this happens so that the previous state is not erased when hydrating a new page. @karaoak mentions that it seemed to happen on only one page for him. I'm not sure why it happens unfortunately, and I can't reproduce it in the example repos.

* Add GIP to _app and add GIP in page to RTK repo

* Added e2e test for RTK repo

* Added testcase for GIAP and GIPP to wrapper

* Consistent casing and formatting in comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Too eager HYDRATION renders SSG pointless.
2 participants