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

shouldComponentHydrate #46

Closed
wants to merge 1 commit into from

Conversation

quantizor
Copy link

@quantizor quantizor commented Apr 27, 2018

View formatted RFC

This RFC seeks to add the capability for a component subtree to opt out of rehydration diffing.

}

shouldComponentHydrate() {
return false;
Copy link

@streamich streamich Apr 27, 2018

Choose a reason for hiding this comment

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

Can it be a static class property instead?

// Stateful:
class CodeSplittingComponent extends React.Component {
  static shouldComponentHydrate = false;
  // ...
}

// Stateless:
const CodeSplittingComponent = () => {/* ... */};

CodeSplittingComponent.shouldComponentHydrate = false;

Copy link
Author

@quantizor quantizor Apr 27, 2018

Choose a reason for hiding this comment

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

It could be, but that would make the functionality less flexible. I really enjoy the potential symmetry with the shouldComponentUpdate lifecycle and was trying to keep usage roughly equivalent (providing a function that returns a boolean based on props.)

@ghost
Copy link

ghost commented Apr 28, 2018

Further motivation for this is content that can't be hydrated at the client at all, e.g. when SSR involves significant work or additional data (such as data lookup, localization or large legacy libraries), or the content itselv is too large and sending a duplicate for hydration is prohibitively expensive.

@ghost
Copy link

ghost commented Apr 28, 2018

Note that the current behavior of dangerouslySetInnerHTML is to keep the current content when the content being set is different. In combination with suppressHydrationWarning it produces the desired behavior already.

<div
    dangerouslySetInnerHTML={{
        __html: ''
    }}
    suppressHydrationWarning
/>

See a working example her: https://codesandbox.io/s/zx38ow3z8x

One obvious alternative is to document this behavior and commit to preserving it.

@tikotzky
Copy link

Seems to be possible to create a component which handles this using the logic from @alexmog2.

It would look like this...

function ConditionallyHydrate({ shouldHydrate = true, children }) {
  if (shouldHydrate) {
    return <div>{children}</div>;
  }
  return (
    <div
      dangerouslySetInnerHTML={{
        __html: ''
      }}
      suppressHydrationWarning
    />
  );
}

Working example here: https://codesandbox.io/s/043n21xn1w

@ghost
Copy link

ghost commented May 25, 2018

The next step in the RFC process is

Build consensus and integrate feedback. RFCs that have broad support are much more likely to make progress than those that don't receive any comments.

It appears that there is a consensus here (of all the people who paid attention so far) that this is a useful performance feature and its implementation is straightforward. What needs to happen for it to become a reality?

@tlays
Copy link

tlays commented May 25, 2018

+1

Can help page load performances & eliminate potential FOUC.
Implementation is straightforward and inline with React lifecycle methods.

All for it

@quantizor
Copy link
Author

Some guidance from core at this point would be useful as to how such a feature should be implemented. Not sure if it’s standard to tag people though

@quantizor
Copy link
Author

quantizor commented May 28, 2018

Note that the current behavior of dangerouslySetInnerHTML is to keep the current content when the content being set is different. In combination with suppressHydrationWarning it produces the desired behavior already.

<div
    dangerouslySetInnerHTML={{
        __html: ''
    }}
    suppressHydrationWarning
/>

^ Very interesting, wasn't aware of suppressHydrationWarning and the non-replacement behavior. I think that method could work for some specific use cases.

What I'm really looking for here is a first-class way to opt out of rehydration, while still allowing the content to potentially change in future render passes. That's the blend of behavior that works best for an SSR->client side async component workflow without unload/load flashing, etc.

@jciolek
Copy link

jciolek commented Sep 4, 2018

@probablyup The documentation says:

"If a single element’s attribute or text content is unavoidably different between the server and the client (for example, a timestamp), you may silence the warning by adding suppressHydrationWarning={true} to the element. It only works one level deep, and is intended to be an escape hatch. Don’t overuse it. Unless it’s text content, React still won’t attempt to patch it up, so it may remain inconsistent until future updates."

I believe this is what you're after?

@quantizor
Copy link
Author

@jciolek actually yes I think that might work.

@itssumitrai
Copy link

suppressHydrationWarning seems to only work for text. Real Ad libraries would most probably modify some DOM nodes, insert images, etc. React would still go ahead and clear out those nodes.

@sebmarkbage
Copy link
Collaborator

I think the innerHTML thing is probably the way to go for now.

For anyone looking for the inverse problem where you only want to render something on the client, Partial Hydration will provide a mechanism for that. The server can render a Suspense boundary with a permanent fallback in the HTML. In fact, I think that's probably what the synchronous server renderer will do by default if the server suspends a component. That will cause the client to throw it out and render the content client side.

You can kind of do the thing where you leave the HTML in place with partial hydration since it doesn't actually have to hydrate everything to be interactive. You can simply suspend by throwing a Promise that never resolves. The issue is that if any Context above that component changes, we'll throw out the content and trigger the fallback. Because that Context might have changed the HTML.

Ultimately, I think we'll want a way to pass rendered content as some kind of JSON object from the server. In a way that can be passed around and just rendered in the tree. Ideally with some embedded child components.

<div>{serverStuff}</div>

If that was server rendered, maybe that data can simply refer to the HTML that was already sent instead of duplicating the HTML.

@gaearon
Copy link
Member

gaearon commented Aug 18, 2021

Thanks for the RFC, and sorry for a very late review (#182).

Quoting the motivation:

When working in a code splitting context, this becomes very important because what is immediately available on the server may not be on the client. A specific example of a typical SSR + code splitting flow:

  • ReactDOMServer renders a complete page to string and sends it to the browser via some means
  • React in the browser rehydrates, but one or more component children in the tree are meant to be asynchronously-loaded in the browser context.
  • When the code splitting component is mounted, this temporarily causes the pre-rendered subtree to be discarded since the asynchronously-loaded child has not yet been downloaded.
  • When the child JS downloads, the code splitting component then enqueues a re-render and the child appears again.

We have a first-class solution to this now, which you can read about here: reactwg/react-18#37. In particular, it allows code splitting to seamlessly work with SSR, and partially hydrate the page as the code loads. There is a demo there that you can play with.

There may be other related problems, so there may still be a space for additional related RFCs. But I'll close this one.

@gaearon gaearon closed this Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants