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

Potential Leak #17

Open
stefanpenner opened this issue Nov 30, 2015 · 10 comments
Open

Potential Leak #17

stefanpenner opened this issue Nov 30, 2015 · 10 comments

Comments

@stefanpenner
Copy link

what uninstalls this from the global.

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

@simonexmachina
Copy link
Owner

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

@stefanpenner
Copy link
Author

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

@simonexmachina
Copy link
Owner

Does appInstance fire a destroy event or something?

@stefanpenner
Copy link
Author

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

@simonexmachina
Copy link
Owner

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

@stefanpenner
Copy link
Author

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.

@simonexmachina
Copy link
Owner

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

@simonexmachina simonexmachina reopened this Dec 3, 2015
@stefanpenner
Copy link
Author

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 ?

@simonexmachina
Copy link
Owner

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
#17 (comment)
.

@robclancy
Copy link

Did he make it back to the office? 😨

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

3 participants