Make React resilient to DOM mutations from Google Translate

[fritz-c]

Coming from search? See workaround here: https://github.com/facebook/react/issues/11538#issuecomment-417504600. And star this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=872770.

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

Bug, though there's a decent chance it's a Chrome/Google Translate one

What is the current behavior?

When using Google Translate on a page using React 16, a certain code pattern produces a Javascript error (Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.) when the rendered content changes.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template for React 16: https://jsfiddle.net/Luktwrdm/, template for React 15: https://jsfiddle.net/hmbg7e9w/).

(This has only been checked on macOS 10.13.1)

1. Navigate to https://qq49kwjynj.codesandbox.io/ in a Chrome browser set to some language other than Japanese.
2. Right click the page and select "Translate to English"
3. Click the checkbox, and the error will show.

The source of the example can be found at https://codesandbox.io/s/qq49kwjynj The part of the code that seems to cause it is the following two lines:

{this.state.checked && "選択済み"}
{!this.state.checked && "無選択"}

Changing this to the following fixes the behavior with Google Translate:

{this.state.checked ? "選択済み" : "無選択"}

What is the expected behavior?

It should not produce an error.

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

I created an identical example with React 15 at the following pages: https://p93xxmr0rq.codesandbox.io/ https://codesandbox.io/s/p93xxmr0rq When repeating the same steps outlined above, no error was produced. It only seems to affect React 16. As this is a Chrome-only feature, it only affects Chrome.

[clemmy]

Thanks for the report - I can also reproduce this bug.

[gaearon]

Could be related? https://github.com/facebook/react/issues/9836

[Tom-Bonnike]

We can also reproduce this bug on our own app. Thanks for the report.

Edit: About the next comment: we also ended up applying this “fix”. It obviously hurts accessibility and users are still triggering this error by right clicking “Translate to X” on Chrome as this meta tag only removes the top-bar translation suggestion.

[MartijnHols]

I "fixed" my app for users running into this by disabling Chrome's translation using this method (until the underlying issue is fixed): https://stackoverflow.com/a/12238414/684353

Edit 2024: Skip to my write-up that covers the entire issue and all of the listed fixes: https://github.com/facebook/react/issues/11538#issuecomment-2136914960 (the one in the OP is broken)

[f0urfingeredfish]

This error also occurs if you use https://localizejs.com for translation. I'm assuming since they manipulate the DOM and React can't reconcile the changes. This took forever to track down and I'm putting it here in case anyone else is trying use localizejs and React16. The issue is with localizejs.

The workaround is to add notranslate tags to the containers of your react components so that localize doesn't mutate the dom. Obviously they will stop getting translated until localizejs comes up with a fix.

  import { render } from 'react-dom'

  const appRoot = document.getElementById('app')
  appRoot.setAttribute('notranslate', true)
  render(<App />, appRoot)

UPDATE Localize.js support confirmed it is a bug on their end and they are working on a fix.

In the meantime this is the solution we are using: https://gist.github.com/f0urfingeredfish/44a3c12b95caf157ad06839ba4b98524

Usage

translate string localize`translate me` translate string but not expressions localize`translate me ${notMe}` translate string and expressions localizeAll`translate me ${andMeToo}` translate expressions aka variables localizeAll`${translateMe}`

[vsiao]

Failed to execute 'removeChild' on 'Node' can be caused by various chrome extensions that replace text on a page. For example, one of our users reported that the "3CX Click to Call" extension was also causing this crash.

@gaearon can you provide any guidance about if and when React could be patched to be resilient against this sort of bug?

[webmobiles]

4 of 7 customers had this problem with react, but it's not happen in all computers, chrome last version 63 working ok for some users but not in others, so it's produces by: no use some keys in react elements, google translate plugins and anothers plugins.. the only solutions is to tell people turn of his plugins...

[hyperknot]

I've just run into this issue as well, which was really hard to figure out. The only clue was that all the strings in Sentry reports were non-English.

How is this not causing problems across all major websites using React 16? Any extension modifying the DOM (which there are a lot, translators, password managers, etc.) can totally break a React website.

The only workaround I found right now is to:

<meta name="google" content="notranslate">

As recommended above.

[hyperknot]

Is there any workaround for this? How is it solved on facebook.com, where Chrome translate works with React 16?

[webmobiles]

it's ok:

with that works, it's means you can't use google translate same time .. but I did realize to repair some errors still without for that I had to implements "key" properties everywhere in my still when react did not complaint about keys absents and another things and another things that i don't remember well, it's about the render, google translate do lost some keys ...

[gaearon]

If you want to help fix this, please create a small reproducing case that doesn't involve extensions, but manually reproduces what they might do to the DOM. Then we can take a look.

[hyperknot]

@gaearon: @fritz-c 's original snippet doesn't involve extensions: https://codesandbox.io/s/qq49kwjynj

Full page here: https://qq49kwjynj.codesandbox.io/

[gaearon]

It involves using Google Translate. I'm asking to create a reproduction case that does what Google Translate would do, but with DOM API calls. So that we can see what exactly is causing the issue.

[hyperknot]

I don't think anyone can answer that, except Google Chrome team. I'm not even sure if Translate is part of the open source Chromium project.

[gaearon]

I don't think you need to know the internals of what Google Translate is doing. Look at DOM before and after for a single word, set some DOM breakpoints, and that should tell you the manipulations necessary to reproduce it. A mutation observer can help too.

[hyperknot]

OK, just simply looking at the DOM it's obvious that it gets changed significantly:

original
<div>無選択</div>

translated:
<div>
  <font style="vertical-align: inherit;">
    <font style="vertical-align: inherit;">No choice</font>
  </font>
</div>

It's interesting that this works well in React 15.

[gaearon]

Right, so I encourage you to write a minimal case in CodeSandbox that does similar mutations and try to reproduce the problem 🙂

[hyperknot]

I did the mutation observers, but haven't been able to replicate it in a way which wouldn't break React 15 as well. I pass it on to a more experienced person from here: https://codesandbox.io/s/lrz2zwp5wl

[fritz-c]

I used Chrome DOM breakpoints to see what Google Translate was doing under the hood, and created a minimal reproduction that closely emulates how it replaces text. Demo: https://5k0q7pl5y4.codesandbox.io/ Source: https://codesandbox.io/s/5k0q7pl5y4

Now with cross-browser compatibility, it breaks in Safari, Firefox and Chrome.

The key lines are at the bottom:

// Get the text node "checked"
const myEl = document.querySelector("div > div > div").childNodes[0];

// Create an arbitrary font element to replace it with
const fontEl = document.createElement("font");

myEl.parentElement.insertBefore(fontEl, myEl);
myEl.parentElement.removeChild(myEl);

This puts the DOM node in a state that will cause the next update by React to throw an error.

I ran the same mutation code in the demo for React 15 (changing .childNodes[0] to .childNodes[1] to select the "checked" text instead of the <!-- react-text: 6 --> comment), and no errors were thrown. So I will declare that this is as close as I can get to a reproduction of the Google Translate behavior.

[gaearon]

Thanks! Any idea why it inserts <font> tags instead of modifying the text node? There's probably a good reason for this but it's not obvious to me.

[fritz-c]

I have no idea. If I had to venture a guess, I'd say it's related to how they incrementally translate large blocks of text (only what's on the screen), and inconsistency between browsers in how bare text nodes are handled.

[shuhei]

Adding a bit more of information. The examples below are based on the ones by @hyperknot and @fritz-c.

The problem is that Google Translate replaces text nodes with <font> tags containing translations while React keeps references to the text nodes that are no longer in the DOM tree.

React throws in the following cases:

1. A text node is conditionally rendered and it's not the only child of its parent. Then React calls parent.removeChild(textNode) when the text node is removed and throws because textNode is no longer a child of parent. https://codesandbox.io/s/74k8lz417x
   • When a conditionally rendered text node is the only child of its parent, React doesn't throw. https://codesandbox.io/s/5kljl5l5yx
2. A node before a text node is conditionally rendered. Then React calls parent.insertBefore(someNode, textNode) when the node is inserted and throws because textNode is no long a child of parent. https://codesandbox.io/s/q7n4mk7m86

// Case 1
<div>
  {condition && 'Welcome'}
  <span>Something</span>
</div>

// Doesn't throw
<div>
  {condition && 'Welcome'}
</div>

// Case 2
<div>
  {condition && <span>Something</span>}
  Welcome
</div>

Workaround

We can avoid these errors by invalidating the conditions above. The easiest workaround is to wrap those text nodes with <span> so that nodes referenced by React will stay in the DOM tree even though their contents are replaced with <font> tags.

// A workaround for case 1
<div>
  {condition && <span>Welcome</span>}
  <span>Something</span>
</div>

// A workaround for case 2
<div>
  {condition && <span>Something</span>}
  <span>Welcome</span>
</div>

[jasonrhodes]

Just curious, has anyone found an existing eslint rule that warns or errors on this? Our Sentry logs have been filled with these errors for a few months now, so it's great to identify the issue. Not sure we'll be able to have our team "remember" to do this convention, though, but it seems lintable.

[jaller94]

What is React’s common behaviour for dealing with modified DOM trees? I assume it is to override the changes and regain certainty about the DOM’s state.

Would it be ok to fully invalidate and rerender the currentParent, if removeChild() fails?

https://github.com/facebook/react/blob/9f78913b20d52a5849dd26aafebfbc3caf190812/packages/react-reconciler/src/ReactFiberCommitWork.js#L721

[gaearon]

@shuhei Thanks for great analysis.

[gaearon]

I guess this worked in React 15 because it inserted the (much derided) comments around text nodes and relied on those comments as a way to hold onto the node position. So even if the text node in between got replaced, React 15 didn't care.

[gaearon]

I don't think it's possible to make React resilient to arbitrary mutations. But if we scope this to being resilient to replacement of text nodes maybe we can make it work.

[gaearon]

Posted a proof of concept in https://github.com/facebook/react/pull/13341

[gaearon]

Soo.. I don't think we can fix this in React without making legitimate mistakes hard to find, or making performance worse for common cases.

But I talked to some Google folks and got this issue filed: https://bugs.chromium.org/p/chromium/issues/detail?id=872770.

If you're affected by this, please feel free to star it and/or share details about how this affects you and why this is important for you to get this fixed.

Finally, there is a workaround you can use that will fix the error. It will make your app a little bit slower, so it is up to you to try it and determine whether the tradeoff is worth it. But it's equivalent to what React would have to do if we were to fix it in React — so you wouldn't have a better solution anyway. The workaround looks like this:

if (typeof Node === 'function' && Node.prototype) {
  const originalRemoveChild = Node.prototype.removeChild;
  Node.prototype.removeChild = function(child) {
    if (child.parentNode !== this) {
      if (console) {
        console.error('Cannot remove a child from a different parent', child, this);
      }
      return child;
    }
    return originalRemoveChild.apply(this, arguments);
  }

  const originalInsertBefore = Node.prototype.insertBefore;
  Node.prototype.insertBefore = function(newNode, referenceNode) {
    if (referenceNode && referenceNode.parentNode !== this) {
      if (console) {
        console.error('Cannot insert before a reference node from a different parent', referenceNode, this);
      }
      return newNode;
    }
    return originalInsertBefore.apply(this, arguments);
  }
}

You can run this code before rendering your application. Note that this will cause some legitimate errors to be just logged to the console (as opposed to being thrown). But it should be sufficient to work around the issues Google Translate is causing in React itself.

Once Google Translate fixes this (if https://bugs.chromium.org/p/chromium/issues/detail?id=872770 gets any traction — it's hard to tell) you'll be able to remove the hack.

Since there's nothing else that I can see actionable here, I'm closing this. Hope it helps!

[benwiley4000]

At the risk of this being a stupid non-performant idea, did you consider this approach:

1. Try to do the normal insertBefore
2. If that doesn't work, check that the expected parent node contains the descendant node
3. If confirmed at step 2, then search up the tree for the ancestor element that is a child of the expected parent node, and use that element instead

[gaearon]

You’re welcome to implement something like this, sure. But we won’t be doing this in React itself because there is overhead to doing even small checks here.

[benwiley4000]

Ok I see

[bsmith-cycorp]

Is this something where it could print a console error without throwing the whole page? I can see some cases where a true exception is warranted - for example, when it wants to add a child node to a node it can no longer find - but trying to remove a node that's already been removed seems non-destructive enough that it could fail gracefully.

[Kamahl19]

@gaearon not sure if you are watching the chromium issue, but there is a really interesting response regarding this https://bugs.chromium.org/p/chromium/issues/detail?id=872770#c9 . What are your thoughts?

[joaovieira]

I can't like that comment enough and I'm impatiently waiting for the outcome.

I myself bumped into this after getting multiple Sentry errors. After reproducing it myself I realized this actually causes the browser to completely crash - horrible for accessibility. As mentioned, relying on engineers remembering the workaround or killing app performance for everyone are not long-term solutions for me.

[jsa-aerial]

@gaearon It is worth noting that this problem is certainly not only google translate. I have hit it using mathjax. Mathjax will take text with latex in it and replace those nodes with something very similar to what google translate is doing. @shuhei 's workaround with the 'span' trick works in the mathjax case as well. I haven't tried your code hack yet, but that would likely be much better.

I will put in my claim that this is something that should be fixed, even if it is just the hack code you give here. As others have pointed out, there are loads of addons that can break React now because of this problem.

If you are worried about performance in the 'typical' case (whatever that may be) you could have this as an option that is setup via some startup request.

[kasperpeulen]

@gaearon As far as I can see, this workaround only solves part of the issue, it makes sure that no errors are thrown. It doesn't make sure that React actually updates the dom in a correct way. In our case this is quite unacceptable, our clients would not be able to see what they actually are going to pay for when google translate is enabled.

Wrapping the react dom tree in a <div className='notranslate'>...</div> solves the issue by disabling google translate. You can then wrap some specific texts in a <span className='translate'>{textToTranslate}</span>, and google will translate those parts. The span by itself also make sure that this issue will not occur.

[rotimi-best]

Can someone please explain the performance issue of the code suggested by @gaearon? It would be helpful to know before suggesting this solution to my team lead. I don't buy the idea of disabling google translate, as that is good for DX but very bad for UX cause it limits who can use my app. And disabling google translate doesn't mean that any other extension can't mutate the dom just as google translate does which triggers the same error again.

[puckey]

@kasperpeulen Did you test your solution? Wrapping the root in 'notranslate' works to stop the translation, but wrapping text in a span with 'translate' className throws the same error as before..

[kasperpeulen]

@puckey Can you show an example to reproduce? We went from 600 errors a day on production to 1 error in a month using this solution. I guess it depends on how "low" you put those <span className='translate'>{textToTranslate}</span>. For example, something like this will probably still break:

<span className='translate'>
  {isFoo ? textToTranslate : ""}
  <br />
  {isBar ? otherTextToTranslate : ""}
</span>`

But if you wrap the textToTranslate directly in a span, it should work.

[kasperpeulen]

@rotimi-best The performance problems are probably negligible. The main problem is that it doesn't solve the issue at all, it just swallows the error. The dom will not mutate correctly with that solution.

[puckey]

@kasparpeulen Thanks for getting back. The problem was indeed a <br/> in a span. Great solution!

[rotimi-best]

@rotimi-best The performance problems are probably negligible. The main problem is that it doesn't solve the issue at all, it just swallows the error. The dom will not mutate correctly with that solution.

Okay I see. Using the span as a work around with a className as 'translate'. This means that everywhere in my app that needs a translation I should add this className to the tag, and a 'notranslate' on my body tag.

Did I get it right?

[kasperpeulen]

@rotimi-best Yes, exactly, and put the class="translate" as low as possible in the tree, or you will get the problem of @puckey. This is some work, but another adventage is that only the stuff that needs to be translated, are getting translated. For example, in the past, some names were also getting translated by google translate, which made users trying to edit their name, as they thought they needed to fix this.

[florentdestremau]

Very glad to find this thread. I fixed the few critical screens that we affected by "simply" nesting my content more aggressively, in particular content with ternary or shorthand display conditions. Here is an extract of the pull request we made:

[dominicfraser]

My understanding is we have two workarounds suggested that are both valid, with different considerations, but only one listed in the top of this issue. I wonder if they both should be @fritz-c, or if we deliberately just list one?

The first by @gaearon that solves/swallows these errors globally, at some performance cost:

https://github.com/facebook/react/issues/11538#issuecomment-417504600

Finally, there is a workaround you can use that will fix the error. It will make your app a little bit slower, so it is up to you to try it and determine whether the tradeoff is worth it. But it's equivalent to what React would have to do if we were to fix it in React — so you wouldn't have a better solution anyway.

Once Google Translate fixes this (if https://bugs.chromium.org/p/chromium/issues/detail?id=872770 gets any traction — it's hard to tell) you'll be able to remove the hack.

The second by @shuhei on a per usage basis, so no performance cost but no guard against reintroducing errors:

https://github.com/facebook/react/issues/11538#issuecomment-390386520

We can avoid these errors by invalidating the conditions above. The easiest workaround is to wrap those text nodes with <span> so that nodes referenced by React will stay in the DOM tree even though their contents are replaced with <font> tags.

[swarshah]

In my case, the text was already rendering in the span tag. However, what worked for me is adding a key attribute to my span tag.

[MidnightDesign]

Another workaroud that worked for me was "disabling" Google Translate entirely by using <html translate="no">.

[PatrickHollweck]

This article might be of interest for anyone facing this issue: https://www.30secondsofcode.org/articles/s/breaking-react

[bernardobelchior]

Just curious, has anyone found an existing eslint rule that warns or errors on this? Our Sentry logs have been filled with these errors for a few months now, so it's great to identify the issue. Not sure we'll be able to have our team "remember" to do this convention, though, but it seems lintable.

@jasonrhodes do you know if there actually is any ESLint rule for this? I tried a quick Google search but couldn't find any