Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add a way to integrate Redux, Apollo, MobX, React Query etc without side effects in render #21932

Closed
Ephem opened this issue Feb 6, 2021 · 10 comments

Comments

@Ephem
Copy link
Contributor

Ephem commented Feb 6, 2021

Describe the feature you'd like to request

Not sure where to post this, as it's also a bug report for a lot of examples, but it's bugs without current bad effects (that I know of, might have them in Concurrent Mode) and requires some new feature to solve, so I opted for a feature request.

Every example of Redux, Apollo, MobX and others relies on side effects in render. React Query has no examples, but the same applies there. I would argue that there is currently no way to integrate these libraries in Next.js without either using getInitialProps (which is not recommended anymore and has drawbacks like no incremental static generation) or relying on side effects in render. Indeed, I don't think there is any way to integrate any library that relies on an external cache without side effects in render.

Examples and more context at the bottom.

Describe the solution you'd like

I'm not sure. Possibly some form of lifecycle that runs before render (prepareComponentForRender(nextProps), but implemented safely as a Next lifecycle). This should work for initial render and on subsequent navigations (in order to be able to hydrate props from the getServerSideProps and getStaticProps into an external cache safely).

I'm honestly not sure a solution exists under the current constraints of Next and React, without providing an API that might be hard to maintain with future versions of React. Maybe a solution in React itself is required (something like side data channels being discussed in the Server Components RFC). I'm also not convinced it's not possible, so I still wanted to file this for smarter people to ponder.

If not fixed, this might very well be a painful blocker for Concurrent Mode (but I'm not sure).

Describe alternatives you've considered

Just adding access to the server and client entrypoints has other drawbacks and wouldn't solve hydration on subsequent navigations anyway.

Doing it in getInitialProps has the drawback of disabling incremental static generation (and possibly other drawbacks).

Doing nothing has the drawback of possibly breaking libraries in Concurrent Mode, possibly with no hope of fixing. Might also be completely fine, but only being able to integrate such popular libraries via side effects in render still doesn't feel right.

Demanding Redux, Apollo, MobX, React Query etc to stop using an external cache and use a component based API (like SWR uses refs) is a big ask.

The broken examples

I think the best way to demonstrate this is to point to a few examples.

with-redux

In with-redux store is getting created, and recreated, in the top level of a hook run in render:

const store = useMemo(() => initializeStore(initialState), [initialState])

As an aside, Redux code splitting without side effects in render also seems impossible in Next at the moment, see this issue: #4010 in Redux

next-redux-wrapper

Most Redux examples (like with-redux-wrapper) uses next-redux-wrapper.

The next-redux-wrapper code is tricky to follow, but in my understanding the store gets created either on getInitialProps if that exists on the page, or in a ref. Doing it in a ref is a side effect (consider starting middlewares e.g.). Even if I've misunderstood store creation, hydration definitely happens in render on the initial render, another side effect.

Don't get me wrong, this is a good library, doing the best it can under the constraints.

with-apollo

The with-apollo example creates a client and hydrates inside of render:

const store = useMemo(() => initializeApollo(state), [state])

with-mobx

The with-mobx example creates a store and hydrates inside of render:

const store = useMemo(() => initializeStore(initialState), [initialState])

End

I could go on and find more examples which rely on these patterns, but my main point isn't that there are broken examples, it's that somewhere in between React, Next and these libraries there exists an incompatibility which forces these implementations to rely on side effects like this in render. I'm confident there simply doesn't exist a way to both avoid getInitialProps and not have side effects in render when integrating these, and no way to support hydration to external caches with getServerSideProps and getStaticProps without doing that inside of render at least on the initial render.

I tried to keep this as succinct as I could but hope it made sense at least to those familiar with the problem space. Feel free to ask questions, I'd love to explain any unclear parts and get a discussion going!

Thanks for reading this far and all your hard work on Next!

Addendum: The Server Components RFC has the same challenge when it comes to hydration.

@todortotev
Copy link
Contributor

todortotev commented Feb 6, 2021

React query has a Next.js example in their own docs so I guess that's why there is no such one here. Last, but not least - no, the same doesn't apply there.

@Ephem
Copy link
Contributor Author

Ephem commented Feb 6, 2021

React query has a Next.js example in their own docs so I guess that's why there is no such one here.

Yeah, I actually explored the full nuance of this problem when working on that exact React Query hydration API. 😄

Last, but not least - no, the same doesn't apply there.

It does. In order to better support Next and create a nice API we actually opted to embrace this flaw and build it into React Query. In React Query the side effect in render happens here:

React.useMemo(() => {
 if (state) {
   hydrate(queryClient, state, optionsRef.current)
 }
}, [queryClient, state])

I should have included that in the original issue, thanks for nudging me!

@Ephem
Copy link
Contributor Author

Ephem commented Feb 6, 2021

I've opted for not including a definition of side effects here. A valid argument could be that creating a store in some cases only creates an object and if you store that on a ref it's actually not a side effect so it might be possible to do that safely. I didn't want to get into those kind of semantics since there are many clear cases of side effects here that can not be solved currently, like hydration.

@todortotev
Copy link
Contributor

todortotev commented Feb 7, 2021

the side effect in render happens here

You don't have to use useHydrate.
https://github.com/tannerlinsley/react-query/blob/master/examples/nextjs/pages/index.js

However, I think your proposal is excellent, and it gives tons of food for thought!

@Ephem
Copy link
Contributor Author

Ephem commented Feb 7, 2021

That example uses the Hydrate component in _app.js which in turn uses the useHydrate behind the scenes, so while a bit hard to follow, you end up with the same side effect in render. 😄

@gaearon
Copy link
Contributor

gaearon commented Feb 7, 2021

I haven’t looked into this issue but wanted to provide the React perspective. It is important to understand that “no side effects in render” refers to observable side effects.

For example, creating an object during render and immediately mutating it is allowed. (Local mutation ftw!) Because you can’t write code that “observes” that the mutation happened outside of that component, or even between its re-renders.

Similarly, lazily initializing a ref during the first time it’s read is allowed. Even though it is a mutation during render, if it’s always checked before assigning, it’s simply indistinguishable from starting out with that value. So it doesn’t break the rules.

Priming a cache generally falls into the same category, as long as the fact that it was primed is unobservable to the components. For example, if there doesn’t exist a codepath where the cache was not primed.

See also: https://gist.github.com/sebmarkbage/75f0838967cd003cd7f9ab938eb1958f#allowed

@gaearon
Copy link
Contributor

gaearon commented Feb 7, 2021

What’s preventing these examples from creating a store during module initialization, outside of the components? This is the part I don’t follow.

@Ephem
Copy link
Contributor Author

Ephem commented Feb 7, 2021

Thanks for the insight, that's helpful! I realize now that I should probably have structured this issue into two clear parts:

Store/cache creation

I think this is the lesser of the problems, and as you say, does seem solvable as long as the libraries have some way of creating a store/cache without also having observable side effects. If they don't, after some thought I now think it should be on the libraries to move side effects to a later phase to allow for pure creation anyway, so this might not be something fundamentally lacking from Next as I implied. I do think some of the examples/libraries still does this in an unsafe way, but it's not impossible to do right.

What’s preventing these examples from creating a store during module initialization, outside of the components? This is the part I don’t follow.

That only runs once on the server, so any store/cache you create there gets shared between requests. This can lead to bugs, but is also a security bad practice as it can leak sensitive data between requests. Store/cache needs to be created per request.

getServerSideProps/getStaticProps hydration

This is where it gets even more tricky and the thing I don't think has a great solution today. Given your explanation of observable side effects, it seems that hydrating these on the very first render is fine, so no issue there. On subsequent navigations however, these gets called for the new page and the props returned, now these libraries need some way of getting those props into the external cache, which is the part that is currently missing.

In this case, doing it in render is a clear side effect, mutating an already existing cache. If you do it in an effect (as I think next-redux-wrapper does) you can end up tearing the tree, since you render the new page with new props, but with the old cache until useEffect has a chance to run. If you block rendering the children until that effect has had a chance to run, you remove the entire tree before rendering the new one, imagine this with going from /article/1 to /article/2 for example, or just with shared layouts in general.

Maaybe you could hack something with a conditional useLayoutEffect on the client to avoid flickering for the user, but you would still tear down the tree and loose any state in it.

This second problem is very similar to the one described on the SC RFC, but because Next owns the lifecycle of when to render the new page, it might be possible to solve within the framework here by adding some way of receiving the props before triggering that render (but this has a whole bunch of implications and might be hard to support in the future, so this API might not be desirable to support for other reasons).

Does that make any sense?

@Ephem
Copy link
Contributor Author

Ephem commented Feb 8, 2021

Just to expand a bit on my reasoning around useEffect, since this is pretty nuanced and I might be missing something. This comment only applies to subsequent renders, since we can hydrate safely in render on the first one.

The reason why I think doing it in useEffect here is weird (but may still be the best currently available option, not sure) is that Next usually guarantees that when a page renders, it already has the props it needs from the server. This may but does not necessarily lead to tearing if the page renders partly with new props and partly with the old cache.

One scenario is when we visit an entirely new page. If the items in the cache are keyed properly, we won't see any old items on the new page, but, we will briefly see loading spinners instead which is also not optimal since we already have the data we need to show.

Another scenario is when we visit a page we've already been to by navigating to it. In Next, (I think) this usually means we wait for new data before rendering that page? If we hydrate to a cache in useEffect we'll briefly show the old data from the cache, before showing the new data.

A third scenario is moving within a dynamic route, for example /article/[id]. If we move from /article/1 to /article/2, I think (but haven't verified) Next doesn't necessarily tear down the entire tree and render a completely new one? If this is the case, hydrating in an effect can unnecessarily tear down (parts of) the tree, since we already have the data needed.

Doing it in a useLayoutEffect (or rather, a custom useIsomorphicLayoutEffect) might solve some of these because it happens before the browser has had a chance to paint, but not all, and doesn't feel quite right anyway.

@Mihai-github
Copy link

Hi,

I came across this problem with apollo client using it for SSR:
"Warning: useLayoutEffect does nothing on the server because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://reactjs.org/link/uselayouteffect-ssr for common fixes."

Has anyone figured out the best way how to resolve this problem because I see there is not a clear and official solution yet?

@vercel vercel locked and limited conversation to collaborators Aug 31, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants