Fix memory leak using custom BUILTIN_MODIFIERS object, fix #28 #29
Conversation
|
I'm not sure if this is a correct fix of memory leak in #28, but it removes it in our application. |
| @@ -95,6 +95,10 @@ import { lte, gte } from 'ember-compatibility-helpers'; | |||
| } | |||
| } | |||
|
|
|||
| runtimeResolver.builtInModifiers = { | |||
There was a problem hiding this comment.
Can you tweak this slightly to:
runtimeResolver.builtInModifiers = Ember.assign({}, runtimeResolver.builtInModifiers);I believe it should have the same benefit (not mutating the global state), but also preserve any other items in the built in modifiers.
There was a problem hiding this comment.
Yes, will do.
BTW I've found another leak with CURLY_COMPONENT_MANAGER few lines below (https://github.com/rwjblue/ember-angle-bracket-invocation-polyfill/blob/master/vendor/angle-bracket-invocation-polyfill/runtime-polyfill.js#L166-L169), I can update this PR (or create a new one) with something like
let customManager = Ember.assign({}, manager);There was a problem hiding this comment.
I've updated the PR, but left CURLY_COMPONENT_MANAGER issue to a separate investigation.
There was a problem hiding this comment.
Ya, updating CURLY_COMPONENT_MANAGER does make sense to me but I'm happy to just review that in a separate PR.
There was a problem hiding this comment.
Yeah, I'll try to provide a separate PR, right now I'm still facing with CURLY_COMPONENT_MANAGER leak, because using Ember.assign breaks setting attributes on components from parent template.
bobisjan
force-pushed
the
memory-leak-fix
branch
from
7ef7df0 to
5c5a010
Compare
|
restarted CI (failures seem unrelated) |
|
wow nice job @bobisjan 🎉 |
No description provided.