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

DOM tree normalization causes React to enter an irrecoverable error state #15014

Closed
paton opened this issue Mar 5, 2019 · 4 comments
Closed

Comments

@paton
Copy link

paton commented Mar 5, 2019

Reproduction:

  1. Open any React site that has a variable adjacent to other text
  2. Type document.body.normalize() into javascript console
  3. React enters an irrecoverable error state
@sebmarkbage
Copy link
Collaborator

Note that the core issue isn’t whether React can break during normalization but whether how much it also breaks for other reasons during the same scenarios normalization is typically used.

For example during translation and phone number finding extensions, text nodes are reordered and/order split across elements in a way that React doesn’t know about.

Adding back the comment nodes doesn’t fix all those issues. We’d have to have data that this will solve a significant chunk of the problems.

Sure, this is a fragile part of React but it is even with the proposed fix.

How can we fix this in the DOM in a way that React can work properly with translations and other extensions in general?

@sebmarkbage
Copy link
Collaborator

One thing that other libraries can do is attach a Shadow DOM to the nodes it wishes to translate. In fact this seems like the perfect use case for Shadow DOM since it hides the implementation details of the real DOM.

@sebmarkbage
Copy link
Collaborator

Look, the general issue here applies to jQuery, lots of libraries and manually written DOM manipulation. The reason you don’t hear about them is because they’re broken in one off cases and nobody goes to report them in a centralized place.

The whole point of the Shadow DOM is because this general problem exists all over interop issues between legacy libraries, extensions, CSS rules etc. Exactly because you can try all you want trying to add defensive coding to libraries but that doesn’t prevent someone writing manual DOM manipulation code to have the same exact problem.

You comment also includes a bunch of irrelevant snark that sound like you’re trying to dismiss us as not valuing or having insight into how our framework is used. That’s not appreciated.

@gaearon
Copy link
Collaborator

gaearon commented Mar 5, 2019

Quoting a React maintainer in the Google Translate Chromium "bug" ticket

Tangential to the topic but I’m not sure why you’re saying the person who opened that ticket is a React maintainer. They’re not. As far as I remember, I reached out to someone who worked on Google Translate to inquire whether there is a possible solution that can be done there, and they opened the ticket. (For which I’m thankful as it started the discussion.)

The 5ms overhead isn't worth the cost (for the 0.01% of React users who use React to power multi-billion dollar news feeds)

I’m sorry if my comments here or somewhere else upset you or gave you the impression that we make these choices lightly or don’t care about end users. I want to personally apologize for that. But the passive aggressive tone in your comments doesn’t contribute to this conversation, and I suggest we don’t carry it further in the comments. Thanks.

@paton paton closed this as completed Mar 5, 2019
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

3 participants