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

Need a hook for hydration mismatch #11189

Closed
redonkulus opened this issue Oct 11, 2017 · 19 comments
Closed

Need a hook for hydration mismatch #11189

redonkulus opened this issue Oct 11, 2017 · 19 comments

Comments

@redonkulus
Copy link

Do you want to request a feature or report a bug?
feature

What is the current behavior?
In React 16, the data-react-checksum attribute was removed from the server rendered markup. In previous versions, we used this attribute to beacon checksum mismatches to our log servers to be notified of production issues. With the attribute removed, we have no mechanism to determine if a checksum mismatch occurred.

I'm aware that checksum issues no longer cause the entire DOM to re-render, however, it is still important that we know when they do occur. A typical use case is when we display ads or autoplay video. We want to know if an ad gets re-rendered (double counted) or an autoplay video is interrupted due to React re-rendering the DOM.

Other related bugs/requests:

What is the expected behavior?
The solution does not necessarily need to re-introduce the checksum attribute again. It could be some other event, hook, or callback that applications can leverage to handle checksum issues.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

  • React 16
  • All browsers
  • Worked in <= React 15
@syranide
Copy link
Contributor

Isn't intercepting and logging warnings (and errors) enough?

@redonkulus
Copy link
Author

redonkulus commented Oct 11, 2017

@syranide like monkey patching console.error and looking for checksum errors? Ideally there would be a properly supported API from React.

One idea could be in the callback from the React.hydrate call? I believe the callback and already return an error object. Perhaps checksum issues could be exposed that way as well.

@sebmarkbage
Copy link
Collaborator

Diffing of attributes and innerHTML no longer happens in the production build so we have no way of issuing this for all possible mismatches. Even for text content we run normalization stuff only in DEV to know whether it actually was a mismatch or not. In theory we could issue some hook for particular mismatches that we actually currently track in production like tag mismatches but it would be very limited since many mismatches doesn't actually end up in different DOM structures.

So you probably have to run the dev build for this to work fully.

A callback to hydrate wouldn't work because we want to be able to asynchronously and lazily hydrate parts of the tree at a time. There is no particular time where we've done all the matching.

Monkey patching console.error seems like the way to do this. Whatever other hooks we expose would end up looking very similar to that. We've talked about an alternative way to intercept warnings in a generic way. Maybe that can be a way.

@redonkulus
Copy link
Author

Thanks for the response @sebmarkbage.

In React 15 today, we do not get the diff in production (as you know), we just know that a mismatch occurred so that we can then run our local dev build to investigate it further. The main concern is just losing the ability to know an error occurred. I'm less concerned with getting the diff in production.

I can try monkey patching, I just don't know if that is the best solution to advocate going forward.

In general, I'm curious how other apps handle this production? Are checksum issues not a concern for most applications? Or because React 16 isolates the checksum issues further down the tree, perhaps its less of an issue?

@lingyan
Copy link

lingyan commented Oct 11, 2017

Is it possible to attach a data attribute (e.g. data-react-mismatch or some better name) on the altered dom node (only the root one would be enough), so that application code can check for reporting purpose?

@redonkulus
Copy link
Author

@sebmarkbage ping

@sebmarkbage sebmarkbage changed the title Need a hook for react checksum issues Need a hook for hydration mismatch Oct 12, 2017
@sebmarkbage
Copy link
Collaborator

Sorry, what I'm saying is that with the new strategy React doesn't even know if something did mismatch or not because we don't actively patch up everything. Not everything is compared.

In general, I'm curious how other apps handle this production?

For React 16, there's not a lot of that use it in production yet. You're an early adopter. :) I think most don't handle it in production and just rely on this showing up at some point during development. A lot of this needs systemic handling by building frameworks around it that ensures that common patterns don't cause this.

@gaearon
Copy link
Collaborator

gaearon commented Jan 2, 2018

Seems like this is the best we can offer:

Monkey patching console.error seems like the way to do this. Whatever other hooks we expose would end up looking very similar to that. We've talked about an alternative way to intercept warnings in a generic way. Maybe that can be a way.

@gaearon gaearon closed this as completed Jan 2, 2018
@redonkulus
Copy link
Author

@gaearon this would only work in development since the console.error messages are not displayed in production builds of React.

@lingyan opened an issue last summer requesting the ability to do this in production but was subsequently closed before the React 16 release.

I believe this feature is still warranted given the scenarios I mentioned in the original description. Correct me if I am wrong, but given fiber knows when the markup is not reused, would it be too much additional work to either add an attribute to the markup (data-markup-reused='false') or provide a hook (either via console.error or some other ability) for apps to listen to?

@gaearon
Copy link
Collaborator

gaearon commented Jan 10, 2018

this would only work in development since the console.error messages are not displayed in production builds of React.

Yes, but as explained above we don’t even know some mismatch cases in production because we don’t do extra work (e.g. comparing attributes). So even if some hook existed it wouldn’t be consistent.

@lingyan
Copy link

lingyan commented Jan 10, 2018

@gaearon Do you mean the extra work is done in development? If that is the case, can we allow applications to enable the "extra work" in production and configure a hook to report? Applications enabling this should understand the performance impact.
If React does not know all the "mismatches" reliably even in development, yes I agree there wont be reliable way to detect the mismatch.

@gaearon
Copy link
Collaborator

gaearon commented Jan 10, 2018

Do you mean the extra work is done in development?

Yes.

If that is the case, can we allow applications to enable the "extra work" in production and configure a hook to report?

I don't really see us adding global configuration options like this. We avoided the need for them for years, and it seems odd for this to be the first one. Ideally we'd want a sane behavior for everyone.

If you feel strongly about it please create an RFC: https://github.com/reactjs/rfcs. If it gets enough support we can consider this.

@OliverJAsh
Copy link

OliverJAsh commented Feb 22, 2018

We're rolling out SSR on unsplash.com and we also need this. Fortunately, because we're still using React v15, we were able to add tracking of checksum mismatches by patching react-dom. Specifically in node_modules/react-dom/lib/ReactMount.js:

         !(container.nodeType !== DOC_NODE_TYPE) ? process.env.NODE_ENV !== 'production' ? invariant(false, 'You\'re trying to render a component to the document using server rendering but the checksum was invalid. This usually means you rendered a different component type or props on the client from the one on the server, or your render() methods are impure. React cannot handle this case due to cross-browser quirks by rendering at the document root. You should look for environment dependent code in your components and ensure the props are the same client and server side:\n%s', difference) : _prodInvariant('42', difference) : void 0;
 
         if (process.env.NODE_ENV !== 'production') {
           process.env.NODE_ENV !== 'production' ? warning(false, 'React attempted to reuse markup in a container but the ' + 'checksum was invalid. This generally means that you are ' + 'using server rendering and the markup generated on the ' + 'server was not what the client was expecting. React injected ' + 'new markup to compensate which works but you have lost many ' + 'of the benefits of server rendering. Instead, figure out ' + 'why the markup being generated is different on the client ' + 'or server:\n%s', difference) : void 0;
+        } else {
+          if (window.onReactProductionChecksumMismatchError !== undefined) {
+            window.onReactProductionChecksumMismatchError({ difference: difference });
+          }
         }
       }
     }

@redonkulus I'm curious how you achieved this using only the attribute. AFAICS, the data-react-checksum attribute is always there, regardless of if there is a mismatch?


I'm not sure how we will manage this going forward, when we do upgrade to React v16, but I think it's quite important we do. From our experience, it's very easy to introduce these errors accidentally.

I think most don't handle it in production and just rely on this showing up at some point during development. A lot of this needs systemic handling by building frameworks around it that ensures that common patterns don't cause this.

@sebmarkbage I completely agree, but in the meantime, in the absence of this infrastructure, we need a way to track these errors because they will inevitably come up.

To make these errors impossible to write, client-only state (e.g. DOM, local storage, window) needs to be impossible to access in the server render and the first-client render.

The way we're thinking of doing this is with a RenderType union type:

RenderType = Unenhanced | Enhanced
  • Unenhanced: the server render and the first client render
  • Enhanced: every client render after the first

All components would receive this union type. window access would only be possible inside Enhanced:

RenderType.match({
  Unenhanced: () => 'data not available',
  Enhanced: ({ window }) => window.localStorage.getItem('foo')
})

@OliverJAsh
Copy link

Using the patch I mentioned for React v15 in #11189 (comment), we've been able to identity many different checksum errors in production that we wouldn't have been able to otherwise. Usually these are checksum errors in browsers we can't test locally, or they happen under some edge case that we just didn't catch when testing locally.

For example, just today I discovered a checksum error in IE and Edge that happens because JS math calculations are different in Node vs IE/Edge, leading to different values for an inline style:

(5342 / 3561) * 100
// => Node: 150.01404099971919
// => IE/Edge: 150.01404099971918

I would have never discovered this without error reporting in production.

Not having a way to report these errors in v16 is a real blocker for us upgrading. We need to be able to monitor these errors when they happen, because they inevitably will.

From this issue it sounds like I'm not alone in needing this. If anyone from the React team could suggest to me how this might work in React v16, I'd be happy to do the work to put together an RFC.

@redonkulus
Copy link
Author

@redonkulus I'm curious how you achieved this using only the attribute. AFAICS, the data-react-checksum attribute is always there, regardless of if there is a mismatch?

@OliverJAsh If the checksum attribute is present then we know the DOM was reused (<= React 15). If its gone, then we know the client re-rendered the whole app. Its simple, but has worked well for us for a few years now.

We too have seen issues in IE based on locale strings, video autoplaying that starts on server rendered markup but then gets re-inited once React on the client starts up (if there is a checksum mismatch), ads that get re-rendered doubling the impression count on markup mismatch. These are major things that require a solution or hook from the library.

I think the trouble we find is that the React team and Facebook do not use React server side so the priority of such issues is lower on the list of other efforts the team is solving. I believe an RFC would be the correct approach, but we need to know what the solution would look like.

@sebmarkbage @gaearon can you help provide a direction on an approach that would be acceptable? We do not know the ins and outs of the React codebase like yourself, so any information helps.

@OliverJAsh
Copy link

I know it's hardly useful to ping on an issue, but it's been awhile, yet this does seem like there is a genuine requirement here. The upgrade to React 16 means we're losing our checksum/mismatch error reporting, which has been very useful in helping us identify all sorts of weird but frequent bugs.

@sebmarkbage @gaearon I'd like to echo @redonkulus's request, could we get some direction from the React team on this?

@kajmagnus
Copy link

kajmagnus commented Jun 16, 2018

What if you compare theReactRootElement.outerHTML with itself, before and after running React? Then maybe you can also find out what the exact mismatch is? (if you do a string diff) — But that doesn't work in all cases, ... if React doesn't always correct the html client side, just reuses the server generated html as is? Here someone writes that attribute mismatches aren't corrected ("doesn’t fix mismatched SSR-generated HTML attributes"), and that React "attempts to change the HTML subtree that doesn’t match" — so comparing the html before and after, would catch some but not all differences?

What if you also compare a hash of the React store json data that I suppose you give to React, when React runs servers side, versus client side? That maybe wouldn't find all errors (not the (5342 / 3561) * 100 // => Node: 150.01404099971919 // => IE/Edge: 150.01404099971918 problem mentioned above), but most of them? (if slightly different react store json is used server side versus client side)

Right now, in my app, I compare a hash of the JSON for the react store, server side, with a hash for the JSON I use client side. They're sometimes a bit different, currently (although they shouldn't be). And if there's a hash mismatch, I use render instead of hydrate. and then print the original HTML from the server to the console, + print the HTML after having run React, and then I can compare the strings & find the problem.

@adamhenson
Copy link

adamhenson commented Jun 30, 2020

Seems like this is the best we can offer:

Monkey patching console.error seems like the way to do this. Whatever other hooks we expose would end up looking very similar to that. We've talked about an alternative way to intercept warnings in a generic way. Maybe that can be a way.

Can someone elaborate on what the above actually means and perhaps hint towards an example? Page experience is becoming a bigger deal than ever and in fact Google will start prioritizing it in search. I help support a website that uses Next.js for server-side rendering and it's apparent that we have a mismatch between client and server renders. I'm having a difficult time in debugging where this mis-match is and if there are more than one. I believe our issue is killing our core Web Vitals as seen in the screenshots below. I've perused Next.js showcase of websites and most of them appear to suffer from the same issue. Would appreciate any detailed suggestions on how to debug... but more specifically clarification on the comments I've referenced at the top.

Before Next.js hydration

Screen Shot 2020-06-30 at 5 07 23 PM

Next.js hydration

Screen Shot 2020-06-30 at 5 07 31 PM

@gaearon
Copy link
Collaborator

gaearon commented Mar 25, 2022

We're adding a way to log non-attribute mismatches in reactjs/rfcs#215. Additionally, the recovery strategy is going to be different and more coarse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants