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

Document that you can't rely on React 16 SSR patching up differences #25

Open
bvaughn opened this issue Oct 6, 2017 · 14 comments
Open

Comments

@bvaughn
Copy link
Contributor

bvaughn commented Oct 6, 2017

This issue was originally filed by @Tarnadas at facebook/react/issues/10591. There's a lot of interesting discussion there.

PR #116 added some additional clarity to the React 16 blog post by calling out that it's dangerous to have missing nodes on the server render, and they might get re-created with incorrect attributes.

@gaearon suggested in a comment below that a core team member should also create a more in-depth blog post about SSR behavior.

@fumblehool
Copy link
Contributor

@bvaughn Can I work on this issue?

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 8, 2017

This issue is all yours! 😄

I've added an "in-progress" label so that others will know not to start work on the issue. If you change your mind about the issue, no worries! Just let me know so that I can remove the label and free it up for someone else to claim.

Cheers!

@bvaughn bvaughn assigned sebmarkbage and unassigned sebmarkbage Oct 10, 2017
bvaughn added a commit that referenced this issue Oct 16, 2017
Fixes #25 - added warning to documentation
@gaearon gaearon reopened this Oct 17, 2017
@gaearon
Copy link
Member

gaearon commented Oct 17, 2017

I don’t think just adding a sentence to the blog post is enough. Originally, we planned to have a whole separate post about this, but @sebmarkbage didn’t find the time to write it.

This is going to keep confusing people over and over, and this behavior should be described somewhere in detail, together with the rationale for it.

@gaearon
Copy link
Member

gaearon commented Oct 17, 2017

We should probably do a post after releasing 16.1.0 that introduces the “suppress warning” feature and explain the new behavior in more detail there. Then we can link to that post from the hydrate API doc.

@sergio-toro
Copy link

@gaearon I totally agree on writing a blog post. I'm just updating our app to 16 and I ran into the SSR breaking design when generated markup does not match.

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 17, 2017

@gaearon Can you please update the title and description of this issue to be more reflective of what you're looking for then? (Or alternately, just open a new issue)

@gaearon
Copy link
Member

gaearon commented Oct 17, 2017

I'm not sure how to update the title. It still describes what I want to happen. I just don't think adding a sentence that's easy to miss to release notes counts as documenting a fundamental change in behavior which hasn't ever changed before (and which people often rely on).

My suggestion is in #25 (comment) but perhaps there are other solutions? I don't want to prescribe a particular one. For new users, it should be something that is easily visible next to our hydrate API reference. For old users, it should be something they can point to and share to educate the community (blog post sounds like a good format). It should include an explanation for why we changed this, and the exact things you can and cannot rely on.

@gaearon
Copy link
Member

gaearon commented Oct 17, 2017

For example, my blog post about attributes provided such rationale and migration advice for the attribute change. It seems like we're not getting issues about the new behavior which means it was well understood.

On the other hand we've gotten five or six issues about SSR behavior. Even with that new sentence, it is too easy to miss or get confused. So it probably deserves its own blog post.

I'm not saying this issue is necessarily actionable for someone else. I see it most as todo for ourselves. No one has more context on this than us. But I want to keep it open until we fix it.

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 17, 2017

Sure, the title's fine I guess. It just seems that I incorrectly summarized what we're looking for and it seems like you have a better idea than I do, and so it may save external contributors time and confusion if we update the description to more clearly reflect what we're looking for. 😄

Even if it's just "a core team member should write a blog post".

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 17, 2017

I took a stab at updating the description and the labels.

@matthewmueller
Copy link

matthewmueller commented Mar 14, 2020

I've been trying to understand what it means for the server & client-side mismatch to be dangerous:

it’s dangerous to have missing nodes on the server render as this might cause sibling nodes to be created with incorrect attributes.

Is there any test or example to show an incorrect rendering?

@gaurav5430
Copy link

Just stumbled into this issue in my nextjs app.
Took me some time to find the original discussion and then this issue.

Any news about the blog post?

jhonmike pushed a commit to jhonmike/reactjs.org that referenced this issue Jul 1, 2020
* Translate strict-mode.md to pt-BR

* Fixes.

* "usando ciclo" -> "que usam ciclos"

Co-Authored-By: lffg <[email protected]>

* "Avisos" -> "Aviso"

Co-Authored-By: lffg <[email protected]>

* "lado negativo" -?

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* reactjs/pt-br.react.dev#25 (comment)

Co-Authored-By: lffg <[email protected]>

* Corrigir "métoods" (reactjs/pt-br.react.dev#25 (comment))
@yangshun
Copy link
Contributor

I think this issue can be closed now as the docs on hydrate() seems to document this behavior well.

@gaurav5430
Copy link

the description for hydrate in the docs does seem to touch on this point, but would be a lot helpful if it talks about some of the pitfalls that can happen because of this. In my case, due to conditional rendering, the order of components was different on server vs client and while patching up hydrate changed the dom structure on the client

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