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

React Query on events page #4

Merged
merged 3 commits into from
Jan 9, 2021
Merged

React Query on events page #4

merged 3 commits into from
Jan 9, 2021

Conversation

kristofferlarberg
Copy link
Contributor

Sets up hydration and refactors events page with React Query

Copy link
Member

@richardolsson richardolsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken a closer look at this and can find no big issues with it. I do have issues with sending the entire pageProps to <Component> (more on that below) but it's not a big deal so we can fix that some other time. I will create a separate issue for it.

Will merge this now. :)

return (
<QueryClientProvider client={ queryClient }>
<Hydrate state={ pageProps.dehydratedState }>
<Component { ...pageProps } />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An unnecessary effect of spreading pageProps like this is that the page component (e.g. OrgEventsPage) will receive the dehydratedState property even though it doesn't need and should never use it directly. The purpose of the dehydratedState property, created on the server by getServerSideProps(), is to provide state to the <Hydrate> component. Once passed to <Hydrate>, the dehydratedState is no longer relevant.

We should exclude dehydratedState from pageProps before passing it to the page component.

A neat looking way to do this would be to use the spread syntax described here (be sure to read the comments as well) to create two variables, dehydratedState and restProps. The former can then be passed to <Hydrate> and ...restProps to <Component>.

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.

2 participants