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

Memory Leak from ArrayProxy.content #238

Closed
brandynbennett opened this issue Jan 17, 2019 · 5 comments
Closed

Memory Leak from ArrayProxy.content #238

brandynbennett opened this issue Jan 17, 2019 · 5 comments

Comments

@brandynbennett
Copy link

brandynbennett commented Jan 17, 2019

Declaring the content property on the notification-messages-service as an array causes a memory leak. Should be declared as null then initialized in init. This started happening after I upgraded from Ember 3.5 to 3.7

content: null,

  init() {
    set(this, 'content', []);
    this._super(...arguments);
  },
@brandynbennett
Copy link
Author

I tried monkey patching with the above code, but it no longer shows notifications when I do this. It seems setting content in init is broken and updates don't get sent to it.

@erichaus
Copy link

@brandynbennett were you able to figure this out?

@erichaus
Copy link

I ended up doing this...

Service.reopenClass({
  content: null,

  init() {
    // prevent memory leak.
    // https://github.com/ynnoj/ember-cli-notifications/issues/238
    this.set('content', A());
    this._super(...arguments);
  },
});

export default Service.reopen({
  addNotification(options) {
    if (!options.message) throw new Error('missing notification message');
   .....

@mansona
Copy link
Owner

mansona commented Nov 12, 2019

@brandynbennett thanks for the report 🎉

I've moved away from using array proxy in the new version #251 (comment) but we are still initialising content in the same way 🤔 does it still cause the memory leak that you're describing? do you have an example of the memory leak?

@brandynbennett
Copy link
Author

Looks like it's gone now. Thanks!

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