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

Give a ssrReconciliation={true} attribute for the developer to ask React to patch up the rendered content if they are not identical between the server and the client #49

Closed
wants to merge 4 commits into from

Conversation

benpptung
Copy link

View RFC

Thank you for any thought.

@sebmarkbage
Copy link
Collaborator

If we had anything that did comparison of everything, I think we would rather throw out the entire subtree rather than patch it up.

There is a real danger here. If we get it wrong while patching up:

  1. We can transfer state that the user has already interacted with. For example a form field. It can be partially filled in but it turns out it didn't line up with the right user/post/shopping item so we patch it up. At the same time, the user hits "enter" and submits the form unwittingly sending their data to the wrong target.

  2. With Partial Hydration, we're probably going to start replaying certain events. The node that was hydrated is used as the target. E.g. for a click. If that target turned out to belong to a different user/post/item than when you clicked, then we'll still issue that click as if the user had clicked the other one.

Therefore it is better to clear out that state than to keep it if we end up getting it wrong. However, what's the scope of getting it wrong? If we throw out the whole tree if we get it wrong it can be jarring a lot of the time as all your simple state like scrolling etc resets. It's also inefficient. That's what we used to do. It gets weirder with Partial Hydration. If we just do it on a single node, then it's not good enough for cases like <div><label for="like">{name}</label> <input type="submit" name="like" /></div>. If we get the name wrong, it's not enough to throw out the label, the place where the error occurred and the thing we need to fix are in completely different parts of the tree.

Take the layout example:

function Foo({wideLayout}) {
  let grid = [
    <div className="row">{children[0]}{children[1]}</div>,
    <div className="row">{children[2]}{children[3]}</div>,
  ];
  let list = [
    <div className="row">{children[0]}</div>,
    <div className="row">{children[1]}</div>,
    <div className="row">{children[2]}</div>,
    <div className="row">{children[3]}</div>,
  ];
  return wideLayout ? grid : list;
}

What if I click the first item in the second row? If I patch that up and then replay the event, I will end up having clicked the wrong item (either 1 or 2 depending on which layout it was).

Maybe scoping it to a whole subtree and a checksum per subtree would work, but would people actually scope it well?

This is made worse by most developers don't consider this such a big deal. I've found it very difficult convincing people of this until you really dig deep into their particular example and why it might break. Seems like React could just figure it out. So if we had this feature, it wouldn't be used in the few places where it might be safe. It would be used all over.

With things like Partial Hydration it is even more important to get things right because the state of the app can change before you even finished hydrating the whole tree.

I think the better strategy here is to actually encourage the two render pass pattern. This is unfortunate if you're doing it synchronously but in the Partial Hydration model you can reactively only hydrate the tree you're interacting with on demand and the first interaction will also be the second render pass that patches up the tree. You have to do two renders to hydrate + render the result of the interaction.

@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.

4 participants