This repository has been archived by the owner on Apr 4, 2019. It is now read-only.
Properly visit MorphLists in visitChildren. #407
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`visitChildren` is used internally to visit all `Morph`'s and call a
callback on each one found. The most common usage of `visitChildren` in
HTMLBars itself is for clearing morphs before rendering a new template.
Prior to this change, if we encounter a node with `morphList` we:
* add the `morphList` to the list of nodes to traverse
* call the provided callback
* traverse the `MorphList` and add all morphs found there to the list of
morphs to call the callback on
In general that is fine, except when the callback itself (in step 2
above) is mutating the `morph` and preventing the `MorphList` from being
traversed. This is the case when changing an `{{if}}` with a `{{each}}`
from the template to the inverse block, in that case the callback is
calling `MorphList#clear()` which nullifies the `MorphList` but does not
call the various hooks (`didDestroyNode` specifically) so any morphs
would be leaked.
This change tweaks `visitChildren` to unroll a `MorphList` upon
encountering it (instead of waiting util after the callback has been
called on it). The included test demonstrates the memory leak that was
occurring in Ember and that it is properly fixed.
|
r+ from @mmun in chat |
rwjblue
added a commit
that referenced
this pull request
Sep 2, 2015
Properly visit MorphLists in visitChildren.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
visitChildrenis used internally to visit allMorph's and call a callback on each one found. The most common usage ofvisitChildrenin HTMLBars itself is for clearing morphs before rendering a new template.Prior to this change, if we encounter a node with
morphListwe:morphListto the list of nodes to traverseMorphListand add all morphs found there to the list ofmorphs to call the callback on
In general that is fine, except when the callback itself (in step 2 above) is mutating the
morphand preventing theMorphListfrom being traversed. This is the case when changing an{{if}}with a{{each}}from the template to the inverse block, in that case the callback is callingMorphList#clear()which nullifies theMorphListbut does not call the various hooks (didDestroyNodespecifically) so any morphs would be leaked.This change tweaks
visitChildrento unroll aMorphListupon encountering it (instead of waiting util after the callback has been called on it). The included test demonstrates the memory leak that was occurring in Ember and that it is properly fixed.