Contextual component tests and basic impl#1
Merged
Serabe merged 1 commit intoSerabe:contextual-componentsfrom Sep 13, 2015
Merged
Contextual component tests and basic impl#1Serabe merged 1 commit intoSerabe:contextual-componentsfrom
Serabe merged 1 commit intoSerabe:contextual-componentsfrom
Conversation
Serabe
added a commit
that referenced
this pull request
Sep 13, 2015
Contextual component tests and basic impl
Serabe
pushed a commit
that referenced
this pull request
Feb 23, 2018
During the Ember 2.15 cycle glimmer-vm was updated and contained a new format for AST plugins. This required that all "legacy" plugins be wrapped (to avoid breakage of this intimate API). When the wrapping code was implemented two distinct bugs were introduced that would have caused custom AST transforms to be called many more times than in Ember < 2.15. Bug #1: Accruing AST Transforms via re-registration --------------------------------------------------- ember-cli-htmlbars allows addons to register AST plugins via the normal ember-cli preprocessor registry system. When this support was added it was not possible to provide plugins during a specific compilation, and therefore all plugins are registered globally (even by current ember-cli-htmlbars versions). Due to the mechanism that ember-cli uses to build up the list of addons and dependencies for use at build time ember-cli-htmlbars ends up requiring the template compiler many times and subsequently registering any AST transform plugins that are present using the exported `registerPlugin` API. Since this tracks plugins in module state it is possible to have `registerPlugin` called many times with the same plugin. ember-cli-htmlbars attempts to mitigate the polution of plugins by forcing the require.cache of the template compiler module to be flushed. Unfortuneately, there are circumstances where this cache invalidation doesn't work and we therefore grow the number of registered AST plugins _very_ quickly (once for each nested addon / engine / app that is in the project). During the 2.15 glimmer-vm upgrade the previous deduping logic was removed (due to the fact that "legacy" AST transforms were always generating unique plugin instances). When combined with situations where ember-cli-htmlbars was not properly clearing the require cache the resulting build times of complex applications using AST plugins grew > double. Bug #2: Legacy AST Transform Compat Bug --------------------------------------------------- In order to avoid breakage in addons leveraging the intimate AST plugin APIs a wrapper layer was added. Unfortunately, the wrapper layer assumed that there would only be a single `Program` node. However, in glimmers AST every `BlockStatement` has a `Program` node (and _may_ have an inverse with another `Program` node). This mistake meant that the legacy "wrapped" AST transforms would be instantiated and ran _many_ times per-template which generates quite a bit of wasted work. Fixes Included -------------- * Bring back the deduplication code ensuring that a given AST plugin is only registered once. * Fix the legacy AST transform wrapper code to _only_ instantiate and invoke the legacy AST transform once for the outermost `Program` node.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
@Serabe this adds an initial tested implementation for things like:
refs emberjs#12285.