Skip to content

fix(EventManager): remove unnecessary dereference#681

Merged
EisenbergEffect merged 1 commit intoaurelia:masterfrom
bigopon:fix-remove-deref
May 28, 2018
Merged

fix(EventManager): remove unnecessary dereference#681
EisenbergEffect merged 1 commit intoaurelia:masterfrom
bigopon:fix-remove-deref

Conversation

@bigopon
Copy link
Member

@bigopon bigopon commented May 4, 2018

No description provided.

@Alexander-Taran
Copy link
Contributor

Can you please comment on why it is unnecessary?

@jdanyow
Copy link
Contributor

jdanyow commented May 6, 2018

possibly unnecessary but have been erring on the side of caution to avoid memory leaks / aid in GC

@bigopon
Copy link
Member Author

bigopon commented May 10, 2018

The class is a disposable object, which could be de-referenced in a single assignment in its consumer. I think it's common practice, hence this PR

@EisenbergEffect
Copy link
Contributor

@bigopon How do we feel? Good to merge this now?

@bigopon
Copy link
Member Author

bigopon commented May 28, 2018

Yes. If its proven to be a source of leaks, we can add this back. I dont know if anyone else is using this new API since the 5th argument needs to be exactly true. And please note that it needs to be disposed exactly the same way with the old subscription: dispose and dereference the subscription, as the old api returns a function that holds reference to all objects used.

@EisenbergEffect EisenbergEffect merged commit 1254764 into aurelia:master May 28, 2018
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

Successfully merging this pull request may close these issues.

4 participants