-
Notifications
You must be signed in to change notification settings - Fork 115
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 Adaptive Hooks will cause attribute mismatch during server side rendering #18
Comments
I don't mind sending a PR as well to fix the issue, but I was wondering what direction you'd want to take (ie, is a 2 phase render type thing ok?) |
😮 are you sure? This sounds like a React bug |
oh wow TIL
Ok, so I propose a function you can call, This is basically what I do, but I use Redux for it so it updates the tree where needed. So it's not a given that this should be solved in this module… |
Thank you for discovering this issue, @gja and for talking through one path forward @wmertens.
My take on
What would be the trade-offs to this approach? Initial render would have to display undefined/some loading placeholder and we update to use the correct values and load media correctly on the next render? |
Yes, the problem is that it is an unnecesary render (and remember that you may generate an entire tree that you remove). This is inevitable if you are using SSR (since your Server Content may be coming from a CDN anyway), but a waste if you are a client only app. My proposed API would look something like this: const { effectiveConnectionType } = useNetworkStatus({ssr: 'loading'});
// How this would work would be something like this
function useNetworkStatusWithSSR({ssr = false}) {
const [loading, setLoading] = useState(true); // <- You can't avoid calling this useState / useEffect here, because of the way react hooks works
useEffect(() => setLoading(false));
if(ssr && loading) {
return { effectiveConnectionType: 'loading' }
} else {
return useNetworkStatus();
}
} |
Another option here would be to export a context provider from the module that enables/disables a "hydration mode". It could switch hooks to provide initial unknown values, then use useEffect or a setState callback to determine when hydration has completed and re-run any hooks that need client-side-specific output. import { AdaptiveProvider } from 'react-adaptive-hooks';
hydrate(
<AdaptiveProvider hydrate={true}>
<App />
</AdaptiveProvider>
); My guess is that emulating the server's "unknown" state would be relatively straightforward since that codepath already exists for SSR. h/t @devknoll for the context suggestion, which is much better than my initial idea of a singleton module with a global isHydrating flag. |
On a related note, I think it would be beneficial to enable the developer to pass a custom value for the initialNetworkStatus. A number of CDNs and other services are able to analyze the user’s connection speed and surface it to the web application server, which the developer can utilize. Of course, there’s no guarantee that this value will always match up with the value derived from the navigator on the client. In instances where the server-side value is reliable and consistent with the navigator value, populating the useNetworkStatus hook on the server can enable adaptive loading on SSR, reduce re-renders, and ensure HTML is consistent between SSR and client-side hydration. |
One benefit of this approach is that it could enable a developer to use Client Hints on the server to detect the effective network connection type then pass this value in to the hook as an initial value (vs opting for I think this is an interesting idea to consider. I would be open to reviewing a PR adding support for this. We may want to consider that Client Hints can be used to expose a range of signals (incl. Device Memory and Save-Data) so this may be something to incorporate for things other than just network. |
I hadn't considered the idea of exporting a context provider that could facilitate a hydration mode. This is a very interesting suggestion, @developit. Could you expand on what you mean by "switch hooks to provide initial unknown values"? i.e are you suggesting we implement something similar to @gja / @mlampedx's proposals of controlling initial defaults if |
Thank you for sharing the client hint documentation -- that is definitely in the vein of what I had in mind. Please find my implementation PR here. |
@mlampedx does your PR solve the issue of hydration having to match the SSR value? I still believe you'll have to do the useEffect / setState hack I mentioned in the beginning of the thread. In such a case, I feel like it's better to pass an explicit value because there is a small impact of render being called twice. |
It does not fix the mismatch issue, which is due to API asymmetry between the client and server, automatically. However, by using client hints to derive a reliable initial value to populate the hooks with on the server, the developer can ensure a consistent hook state and avoid this mismatch. In cases where this option is unavailable to the developer, I think that we should either:
E.g.
or Use the |
These are the cases to cover:
1. Server-side rendering
2. Client-side hydration
3. Client-side first render without hydration
4. Client-side subsequent renders
In 1, the values need to be guessed at so they are consistent with the
client.
In 2, the values need to be those used on the server, even if they are
different from the client.
In 3 and 4, the values should be the measured ones, either static or
dynamic.
The AdaptiveProvider can set this up: it provides an AdaptiveContext that
handles the SSR states, and if it's not used, the AdaptiveContext defaults
are the measured values.
By using a context, measurement happens only once, and all consumers are
updated automatically.
So, the static values should be measured once and used as the default value
for the AdaptiveContext.
AdaptiveProvider gets guesses for the client values (SSR) or the
server-guessed values (provided via some side channel), and uses those for
the first render. On subsequent renders, it sets the context to the
measured values.
For the SSR guesses side channel, one option would be to emit the guesses
as data-x attributes which are read from the DOM on client load, but
perhaps that is too invasive.
|
Hi. I saw a recent issue #2 regarding server side rendering. However, this behaviour simply hides the attribute mismatch that can happen due to the SSR phase and the client phase rendering differently.
I have built an example repo that showcases this mismatch. https://github.com/gja/react-adaptive-hooks-ssr
Apologies for just copying the relevant file (react-adaptive-hooks/network), but I didn't have time to configure babel and whatnot for server side rendering.
Usage to start repo:
As you can see from https://github.com/gja/react-adaptive-hooks-ssr/blob/master/src/ReactAdaptiveHooksExample.js, it should render a paragraph whose text and class are matching (ie:
<p class="4g">Your Network Status Is 4g</p>
). However, the ssr hydration will not resolve the classname (ie:<p class="unsupported">Your Network Status Is 4g</p>
).However, react hydrate does not go through element attributes to look for mismatches, hence these attributes will never be caught. (worse, react believes that the classname is set to 4g, while the dom has a different value, so this will not be rectified in future renders as well).
I'm not sure what the supported model is here. In the old class world, i'd have only called out to the new functions during componentDidMount, and change the behaviour that way. I'm not sure what the equivalent is in the react hooks world.
Maybe return undefined when loading, then useEffect to set loading to false, and return the correct value on the next render.
The text was updated successfully, but these errors were encountered: