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

Should glimmer catch DOM NotFoundErrors when removing elements? #736

Closed
reidab opened this issue Nov 6, 2017 · 1 comment
Closed

Should glimmer catch DOM NotFoundErrors when removing elements? #736

reidab opened this issue Nov 6, 2017 · 1 comment

Comments

@reidab
Copy link

reidab commented Nov 6, 2017

I recently spent some time debugging a rash of Cannot start a nested transaction errors like those described in #484. Specifically (like #484 (comment)) these errors were fallout from an uncaught NotFoundError DOMException raised when Bounds#clear tried to remove a node from the DOM that had already been removed.

What turned out to be happening was:

  1. Our Ember app used ember-cli-head to inject a {{head-layout}} into the document <head>.
  2. Some code we'd extracted to a lazily-loaded Ember engine also included ember-cli-head, so this sneaky instance-initializer ran when the engine loaded and gleefully smashed away all the {{head-layout}} DOM nodes.
  3. The next attempt to update the {{head-layout}} component failed (presumably because the DOM was gone)
  4. The VM frame's exception handler attempts to clear a set of bounds where the bounding elements had both been removed by the instance-initializer in step 2.
  5. 💥 DOMException: NotFoundError

While this is somewhat of an edge case, it leads me to think that the glimmer VM might want to handle the possibility of DOM-gone-missing whenever it uses removeChild.

Does it make sense to catch these DOMExceptions, since they seem to indicate that the removal has already happened? Or maybe to guard against them by verifying the node's parent is set before calling removeChild?

@chancancode
Copy link
Contributor

Sorry for the late response, this was brought to my attention on Discord today.

I think this can be closed now, as ember-fastboot/ember-cli-head#45 seems to have addressed the issue in ember-cli-head. Though definitely not perfect, recent versions of Ember (which brings newer versions of glimmer-vm) also has better recovery/error messages around these error cases (improvements to the error messages are always welcomed).

However, I think this is a genuine error in the user code and we shouldn't just silently ignore it. Essentially a piece of glimmer-vm owned/managed DOM was mutated/removed by foreign code and that's generally not supported/undefined behavior. It seems like ember-cli-head was able to avoid triggering this scenario in the first place which is the right thing to do.

This error (which could be less cryptic) is just one of the symptoms caused by the unsupported/unexpected DOM mutation. Even if we silenced it here, it will likely cause other problems (and be even more difficult to debug when it inadvertently moves the problem somewhere else).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants