[3.17] [Bug] didInsert / willDestroy Firing Out Of Order

[MatthewPringle]

I have 2 routes, each with 1 component. Each component uses the {{did-insert}} render modifier and the willDestroy action.

Problem When transitioning routes the did-insert action for the new component, on the new route fires before the willDestroy action of the old component on the old route.

I would expect the old component, on the route I am transitioning from, to be destroyed before the new component on the route I am transitioning to fires {{did-insert}}

Reproduction Scenario I have created an Ember Twiddle to illustrate the problem https://ember-twiddle.com/2412fd6f6549a66ae4e20c3cb0613858?openFiles=components.scene-2%5C.js%2Ctemplates.components.scene-2%5C.hbs

You will need to use the developer console to see the output. Which should look like the below, which happens when transitioning from Index -> Page 2

[Log] Scene 1 - didInsert [Log] Scene 2 - didInsert [Log] Scene 1 - willDestroy

[pzuraq]

This is expected behavior. Destructors are scheduled to happen asynchronously, which means that their timing relative to other hooks is not guaranteed. willDestroy in particular has always been scheduled to happen with these semantics.

Stepping back, destructors are really meant to work like garbage collection in a sense - they're for doing cleanup that needs to happen eventually, for things like avoiding memory leaks, etc. but they shouldn't need to happen immediately for any reason. Ideally, you would be able to schedule them to run with requestIdleCallback so they don't block render in any way, but I expect a lot of destructors do expect to run sometime before paint so this will probably be something we'd have to introduce over time.

The willDestroyElement hook on classic components does work they way you describe, however. Currently there is no equivalent with Glimmer components or modifiers, as we were waiting for compelling use cases. Recently we found at least one, which was the ember-css-transitions library. I brought this up with the core team recently, and we agreed that a hook should be added to modifier managers to make synchronous destructors a possibility. I haven't had a chance to write an RFC, but definitely would be open to pairing with someone on it!

More compelling use cases would also be helpful. In this case, I don't think adding/removing items from a scene is particularly compelling - it shouldn't matter what order the items are added or removed in typically.

[MatthewPringle]

The use case...

Phaser.io or babylon.js game engines use a canvas to draw WebGL.

Let say for each level in a game that's managed by an Ember app and uses one of those engines, I need to clear the scene and setup the next scene. My idea was to transition into a route, one for each level / scene and have the route setup the level on did-insert.

WebGL doesn't like the canvas being teared down and recreated for each scene in a single page app, after a while it starts generating errors about "Too many active WebGL contexts. Oldest context will be lost." which I can't solve. Im guessing this is not a good thing though, even if I am not using the old contexts any more.

So instead of each level rending a canvas through including a canvas component and passing the level object as a param the recommended solution is to use one canvas and pass the level object through a service.

So that's what I did, having the level register with the service and then monitoring the level in the service. Once the level completes and the level hands back to the service the app transitions to the next level and it starts over.

Unfortunately as the did-insert fires before the willDestroy the new level starts setting up the canvas before the last level has teared down. This crashed the game engine.

I was using this as a way of learning Ember Octane and previously I would have used willDestroyElement, infact I do use willDestroyElement a lot in my apps, as well as the previous Init / didInsertElement.

I realise you want as much to be done in the templates as possible but sometimes it isn't possible, especially when working with imported packages. For instance I use bloodhound.js for an autocomplete element and everything works fine in Chrome passing in params, but in Safari on OSX I need to use a Ember.run.later() function to setup bloodhound. That seems to be a Safari bug, but one I used ember's component lifecycle to fix.

Why put a game engine in Ember? I wrote an npm package to create Ember / Capacitor apps so its easy to build iOS and Android apps with Ember. https://www.npmjs.com/package/ember-cap and so I was wondering how easy it would be to build an Ember Octane / Capacitor / iOS game.

[pzuraq]

The more detailed breakdown does make the use case more compelling, so I think we could add it to the RFC to add this functionality. We hadn't found an example of a library that would require these timing semantics, but expected at least one existed. Thank you for taking the time to write this up!

I realise you want as much to be done in the templates as possible but sometimes it isn't possible, especially when working with imported packages.

Do you think that the ability for modifiers to run code before their element is removed would satisfy your use case here?

[MatthewPringle]

Do you think that the ability for modifiers to run code before their element is removed would satisfy your use case here?

That would depend if the modifier ran before any did-insert modifiers ran on the new route.

I can obviously add extra code into my app when programmatically transitioning and extra code on the routes to catch any other transitions so I can always tear down the canvas before the new route is entered.

But I would have expected a way of doing like the way willDestroyElement worked in Ember classic.

[pzuraq]

It would necessarily run before did-insert ran. Alright, sounds good 👍

[boris-petrov]

@pzuraq - I have another example if you need it. Concerning a third-party library (but the problem is not in it I believe but how browsers work). We use dashjs for playing video files. In a will-destroy hook (via a modifier) I'm doing player.dispose() which should clean up stuff. It blows up with:

DOMException: Failed to set the 'appendWindowEnd' property on 'SourceBuffer': This SourceBuffer has been removed from the parent media source.

Because the element is no longer in the DOM. There is literally nothing I can do to "fix" that. I have to either not dispose of the player or ignore the error. This can only be fixed by synchronous destructors.

Besides, why did you name the modifier will-destroy and let it run after the element has been destroyed?

[pzuraq]

Like I've mentioned above, I have raised this issue with core and there is general consensus that we need to add a capability to modifiers for running destruction code before the element is destroyed. This should be in the form of a new lifecycle hook, not replacing the current destroyModifier hook.

I'm currently very busy and not sure when I will have time to write the RFC for that, and would definitely appreciate help getting it drafted and up. If anyone has the time, let me know!

[simonihmig]

Let me chime in here. So my concerns are also related to WebGL rendering, but could be relevant for other (DOM-less) use cases as well...

destructors are really meant to work like garbage collection in a sense - they're for doing cleanup that needs to happen eventually, for things like avoiding memory leaks, etc. but they shouldn't need to happen immediately for any reason

I didn't experience problems so far related to willDestroy timing, but this makes me a bit nervous. So in the case of wrapping 3D stuff with components, the timing of willDestroy seems highly relevant. So for example you have something like:

<Scene>
  <Mesh mesh={{this.mesh}}/>
</Scene>

The Mesh component should of course not render anything to DOM, but modify the scene tree. So in the constructor it will add a new mesh node to the scene, and on willDestroy in will remove it (and eventually other cleanup work like explicitly disposing the mesh instance to free allocated memory from the GPU). But in this case this must happen instantly (or at least in the next microtask/event loop, but not just some time), otherwise the 3D engine will render the scene in the next rAF with a mesh that shouldn't be there anymore.

And as we are talking about DOM-less rendering in this example here, this means we would need that timing guarantee for components, not a modifier. (as you cannot use modifiers without DOM, see https://github.com/emberjs/rfcs/issues/597)

[simonihmig]

After thinking about this a bit more, I realized I might have confused things a bit...

@pzuraq by saying that destructors are scheduled to happen asynchronously, do you mean that's just how the semantics of the Glimmer component manager happen to be (I believe here: https://github.com/glimmerjs/glimmer.js/blob/master/packages/%40glimmer/component/addon/-private/ember-component-manager.ts#L53), or is this always the case?

Or to rephrase: can I expect that destroyComponent of a component manager is called synchronously? In which case I could make sure that the timing semantics match what I would need in another world (3D vs. DOM)

[rwjblue]

The current semantics of destroy should match the semantics defined in the destroyables RFC I think.

[robclancy]

I used to have no problems with did-insert and will-destroy modifiers working properly. Now I get issues with destroy happening after so multiple features have broken.

One component has tabs, and when we change a category those tabs change. When you insert the first tab it is set to active automatically. When a tab is destroyed and it was active it will unset the active. So when changing category, which changes all the tabs used, it would unset active so then when the first insert happens the tab would be activated.

This worked up until recently. I don't know if it was just luck or a slower machine so the async happened earlier.

Also how you describe it working (by design) isn't intuitive at all for developers. It sounds like the only use case it was designed for was using 3rd party tools and cleaning them up.

[pzuraq]

@robclancy like I mentioned above, we have determined that there are use cases for both eager destruction (which blocks rendering) and lazy destruction (which does not). We are very open to adding a willDestroyModifier type hook and capability to modifier managers, we just need an RFC, and so far no one has had the time to step up and write it.

If you're interested in getting this functionality, I'm happy to help with writing and reviewing the RFC and getting it through the process. It's on my backlog to do it myself, but I'm unsure about when I will have time to get it done.

I'll also note that I think you could probably model that component's data flow without needing to use did-insert or will-destroy at all. For example, you could use a registration pattern, where the parent tabs component controls the state of the child tabs directly rather than relying on the timing semantics of lifecycle hooks like this. This is what I meant when I said the majority of use cases for destruction are for garbage collection - most other use cases have better alternatives. See this twiddle for an example: https://ember-twiddle.com/c60bac6f124181473d3ddc37ed267df4

@simonihmig currently destructors run after rendering rather than mid render, which was the change that caused issues here. Timing semantics are clearly important (having caused issues already here, even though we made a change that was allowed and expected in the RFC itself), so it is very unlikely we would just change this and start running destructors after paint some time. I believe it would probably need to be rolled out via an optional feature flag, or via a new capability. The main point is this is where we would like to push destruction, because the majority of use cases for destroying don't need to be sync, and the few that do should be able to opt-in to the more expensive sync destruction, ideally.

[snewcomer]

we were waiting for compelling use cases

I am very willing and motivated to champion something here. Given the backlog of 4.0 things, I'd be happy to it after 4.0 is released. This is a bug often coming up when migrating.