Add failing tests for resilience to Google Translate

[gaearon]

While #13341 didn't work out I figured I'd share the test cases I used with it. The test code is kinda wonky — it's adapted from our text node permutation tests, but with some twists to simulate what a mutation should do in each case.

[nhunzaker]

Thank you for sending this out! This is super helpful.

Right now I'm playing around with passing the index of the Fiber to the host config. React DOM can either:

commitText:

export function commitTextUpdate(
  textInstance: TextInstance,
  oldText: string,
  newText: string,
  parent,
  index,
): void {
  let actualNode = parent.childNodes[index]

  if (actualNode !== textInstance) {
    actualNode.replaceWith(textInstance);
  }

  textInstance.nodeValue = newText;
}

Essentially: check to see if the DOM has what we expect. If not, fix it before applying the update.

removeChild

export function removeChild(
  parentInstance: Instance,
  index: number,
): void {
  let child = parentInstance.childNodes[index]
  parentInstance.removeChild(child);
}

Similarly. Don't remove a child that could have been replaced. Assuming we want to remove the child anyway, just remove the item at the current index.

insertInContainerBefore

export function insertInContainerBefore(
  container: Container,
  child: Instance | TextInstance,
  beforeChild: Instance | TextInstance,
  index: Number
): void {
  let actualNode = container.childNodes[index]

  if (actualNode !== beforeChild) {
    actualNode.replceWith(beforeChild)
  }

  if (container.nodeType === COMMENT_NODE) {
    (container.parentNode: any).insertBefore(child, beforeChild);
  } else {
    container.insertBefore(child, beforeChild);
  }
}

This behaves just like commitText

---

This approach actually fixes a few of the tests in this PR that assert incorrect markup. Like here.

For fun, I'm calling this self healing.

It works well for Google Translate because it swaps out the translated nodes in-place, which (as far as I can tell) triggers the MutationObserver bound to the parent. Text is magically re-translated.

The problem with this approach is that the index for Fibers isn't updated as they are removed: as far as I can tell, this only happens after they are committed. It breaks down when you remove nodes, particularly for clearing all nodes, like:

// Fiber indexes: 0, 1, 2
ReactDOM.render(<div>{'one'}{'two'}{'three'}</div>)
ReactDOM.render(<div />)

// Steps:
// Remove 0
// Try to remove 1, but the index is still at 1 even though the prior sibling was removed from the DOM
// Try to remove 2, but we're past the index because we've technically removed nodes 0 and 2
// DOM Error: removeChild was not given a Node

What if we made it so that Deletion side-effects updated the index of sibling nodes?

[gaearon]

Fiber index won't help you here because it's fiber index, not a host node index.
For example in

<div>
  <React.Fragment>
    <Child1>
      <div />
    </Child1>
  </React.Fragment>
  <div />
  <React.Fragment>
    <div /> <!-- fiber.index is 0, but hostIndex is 2 -->
  </React.Fragment>
</div>

the last div fiber's index will be 0 (because it's the first fiber inside its parent fiber), but the host index is actually 2 (it's the third div inside its parent div).

[nhunzaker]

Fiber index won't help you here because it's fiber index, not a host node index.

Right. Would it be practical to track the host node index? If so, is there merit in the approach I've laid out?

[gaearon]

How would you track it without incurring overhead for every single operation?

[nhunzaker]

There would be overhead 😞 for most operations. Both to track the index, and to check for correctness.

I wonder if MutationObserver is a good candidate here. It would watch insertions/deletions and to track unexpected mutations. Those DOM nodes could be placed in a map of "tainted" elements, which could be swapped back into the DOM when React needed to next operate on them.

That would have its own overhead, but, speculatively, it could look like:

const taintedNodes = new Map()

let guardian = new MutationObserver(mutationList => {
  mutationList.forEach(mutation => {
    switch(mutation.type) {
      case 'childList':
        // map added items to removed items when added items are not
        // tracked by React
        // ....
        taintedNodes.add(removedNode, addedNode)
        break;
    }
  })
})

guardian.observe(root, { attributes: false, childList: true, subtree: false })

function heal(node) {
  if (taintedNodes.has(node)) {
    // key is the child, value is the node that replaced it
    taintedNodes.get(node).replaceWith(node) 
    taintedNodes.delete(node)
  }
}

I'm going to play around with it. Unfortunately, this adds overhead to every insertion/deletion too.

[nhunzaker]

Just following up, I started down this path and quickly discovered a few issues:

• JSDOM does not support MutationObserver
• Mutations often come through as separate add and remove mutations, and it is hard to figure out of a mutation was from replaceChild or separate addChild and removeChild calls

There might be something there, but it feels really mechanical and fraught with error. Back to the drawing board.

[gaearon]

If someone is looking for workaround: #11538 (comment)