ember-devtools icon indicating copy to clipboard operation
ember-devtools copied to clipboard

Potential Leak

Open stefanpenner opened this issue 10 years ago • 10 comments

what uninstalls this from the global.

https://github.com/aexmachina/ember-devtools/blob/master/app/instance-initializers/ember-devtools.js#L14

stefanpenner avatar Nov 30 '15 20:11 stefanpenner

Nothing. Is there an uninitialize hook I can use to remove it?

simonexmachina avatar Dec 03 '15 00:12 simonexmachina

unfortunately not, it would be really nice if initializers had deinitializers. Especially for test run related cleanup. My typical approach is to do the cleanup when the app is destroyed

cc @dgeb

stefanpenner avatar Dec 03 '15 00:12 stefanpenner

Does appInstance fire a destroy event or something?

simonexmachina avatar Dec 03 '15 01:12 simonexmachina

right now it requires a reopen + willDestroy or destroy override + super to work correctly, which is extremely crappy :sadpanda:

stefanpenner avatar Dec 03 '15 01:12 stefanpenner

Okay, I think this is a NOOP. Registering a global is inherently dirty anyway, it's only supported for convenience during development.

simonexmachina avatar Dec 03 '15 01:12 simonexmachina

Okay, I think this is a NOOP.

This leaks an app during tests, which means it masks other leaks when hunting for leaks.

I believe the reopen is crappy, but fine until we support something better.

stefanpenner avatar Dec 03 '15 01:12 stefanpenner

I see. Okay, how can I do the reopen?

simonexmachina avatar Dec 03 '15 01:12 simonexmachina

in the same file as the initializer:

Ember.Application.reopen({
   destroy() {
     this._super(...arguments);
     /* cleanup */
   }
})

fugly but should do the trick, deinitialize seems mega important. Maybe a post vacation task for myself, unless i can convince señor @dgeb or @twokul ?

stefanpenner avatar Dec 03 '15 01:12 stefanpenner

Oh man that's not nice. Okay I'll do it when I get back to the office (with gloves on). I'm gonna put your name on it Stef ;) On Thu, 3 Dec 2015 at 12:36, Stefan Penner [email protected] wrote:

in the same file as the initializer:

Ember.Application.reopen({ destroy() { this._super(...arguments); /* cleanup */ } })

fugly but should do the trick, deinitialize seems mega important. Maybe a post vacation task for myself, unless i can convince señor @dgeb https://github.com/dgeb

— Reply to this email directly or view it on GitHub https://github.com/aexmachina/ember-devtools/issues/17#issuecomment-161488147 .

simonexmachina avatar Dec 03 '15 01:12 simonexmachina

Did he make it back to the office? :fearful:

robclancy avatar Dec 26 '17 15:12 robclancy