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

removeWormhole triggers error in Ember 3.22 #70

Open
jacobq opened this issue Oct 19, 2020 · 5 comments
Open

removeWormhole triggers error in Ember 3.22 #70

jacobq opened this issue Oct 19, 2020 · 5 comments

Comments

@jacobq
Copy link

jacobq commented Oct 19, 2020

This appears related to yapplabs/ember-modal-dialog#312

removeWormhole(wormhole) {
const stackName = wormhole.get('stack');
const stack = this.stackMap.get(stackName);
const item = stack.find(item => item && item.wormhole === wormhole);
const newNodes = item.get('nodes').clone();
item.set('nodes', newNodes);
item.set('_replaceNodes', true);
next(() => stack.removeObject(item));
},

I don't know yet if the problem is here in liquid-wormhole or ember-modal-dialog where it is used, but when upgrading to Ember 3.22 I started running into errors like the following (see also emberjs/ember.js#19110):

index.js:172 Uncaught Error: Assertion Failed: You attempted to update `nodes` on `<EmberObject:ember169>`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

`nodes` was first used:

- While rendering:
  application
    animatable

Stack trace for the update:
    at dirtyTagFor (validator.js:544)
    at markObjectAsDirty (index.js:546)
    at notifyPropertyChange (index.js:584)
    at set (index.js:1526)
    at EmberObject.set (observable.js:176)
    at Class.removeWormhole (liquid-destination.js:66)
    at Class.removeWormhole (liquid-wormhole.js:35)
assert @ index.js:172
assert @ index.js:5122
assertTagNotConsumed @ validator.js:234
dirtyTagFor @ validator.js:544
markObjectAsDirty @ index.js:546
notifyPropertyChange @ index.js:584
set @ index.js:1526
set @ observable.js:176
removeWormhole @ liquid-destination.js:66
removeWormhole @ liquid-wormhole.js:35
willDestroyElement @ liquid-wormhole.js:53
superWrapper @ index.js:442
trigger @ core_view.js:70
superWrapper @ index.js:442
willDestroy @ index.js:2942
(anonymous) @ index.js:2931
(anonymous) @ runtime.js:752
iterate @ runtime.js:644
destroy @ runtime.js:752
iterate @ runtime.js:641
destroy @ runtime.js:751
iterate @ runtime.js:641
destroy @ runtime.js:751
iterate @ runtime.js:641
destroy @ runtime.js:751
iterate @ runtime.js:644
destroyChildren @ runtime.js:778
handleException @ runtime.js:5105
handleException @ runtime.js:5344
throw @ runtime.js:5041
evaluate @ runtime.js:2348
execute @ runtime.js:5020
rerender @ runtime.js:5372
render @ index.js:8659
(anonymous) @ index.js:8946
inTransaction @ runtime.js:4280
_renderRoots @ index.js:8926
_renderRootsTransaction @ index.js:8978
_revalidate @ index.js:9020
invoke @ backburner.js:338
flush @ backburner.js:229
flush @ backburner.js:426
_end @ backburner.js:960
end @ backburner.js:710
_run @ backburner.js:1015
_join @ backburner.js:989
join @ backburner.js:760
join @ index.js:168
handler @ index.js:6837
(anonymous) @ event_dispatcher.js:352
dispatch @ jquery.js:5429
elemData.handle @ jquery.js:5233
Show 22 more frames
@pzuraq
Copy link
Owner

pzuraq commented Oct 20, 2020

I unfortunately don't have time to continue working on this addon, but if anyone would like to solve it PRs are welcome!

@jacobq
Copy link
Author

jacobq commented Oct 21, 2020

I unfortunately don't have time to continue working on this addon, but if anyone would like to solve it PRs are welcome!

Bummer :'(
I will try to at least get more info here (e.g. reproduction steps and any guesses/insights related to the problem), but I suspect I won't know how to properly fix it so will probably just downgrade my app back to the previous Ember release until someone fixes it or I migrate off of ember-modal-dialog.

@jacobq
Copy link
Author

jacobq commented Oct 21, 2020

  • Forked jacobq/liquid-wormhole
  • Had trouble even running yarn (using Windows & node-sass wouldn't compile with the build tools I have)
    so did massive/blind upgrade via ember-cli-update + npm-check-updates + yarn upgrade
  • An error like the one alluded to above can be seen in several of the test now, e.g. ember test --server --filter="destination container is cleaned when empty"

image

jacobq added a commit to jacobq/liquid-wormhole that referenced this issue Oct 21, 2020
Looks like `removeWormhole` can get called in the same cycle as
`didInsertELement`, and both touch `item.nodes`, which triggers
an error in Ember 3.22 that looks something like this:

```
Uncaught Error: Assertion Failed: You attempted to update `nodes` on `<EmberObject:ember169>`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

`nodes` was first used:
....
```

To work around this for now (probably not the best way) we postpone the
update to the next turn of the rendering loop.
@jacobq
Copy link
Author

jacobq commented Oct 21, 2020

@pzuraq If you won't have time to maintain this addon any more, perhaps you'd like to migrate it to adopted-ember-addons? I don't know how much usage it is seeing in current apps, so am not sure if it'd make more sense to just deprecate / put notice about no longer being maintained etc....

@pzuraq
Copy link
Owner

pzuraq commented Oct 21, 2020

Yeah that sounds like a good idea, I'll reach out to them

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