Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAvoid require cache busting whenever possible. #130
Conversation
This test demonstrates how each "layer" in an ember app is transpiling its own templates. Each is _supposed_ to be able to provide its own AST plugins, **but** this test shows how this falls down and fails. The current mechanism for registering AST plugins is fundamentally flawed. This forced us (long ago) to add some shenanigans to clear the require cache (to avoid this exact state leakage).
In order to stop the cache busting system, we need to: * Update the actual compilation code to pass in our custom options (done in a prior commit) * Ensure that **no one** is mutating the global list of plugins (if they are, then we will always get their plugins and therefore our compilation is "spoiled" by their AST plugins) * Avoid purging the `require.cache` when the requirements are met
|
LGTM - this brings an end to an era. I still remember us debugging this originally, thanks for tackling this! |
| @@ -109,12 +187,24 @@ module.exports = { | |||
| pluginCacheKey: pluginInfo.cacheKeys | |||
| }; | |||
|
|
|||
| if (this.legacyPluginRegistrationCacheBustingRequired !== false) { | |||
stefanpenner
Oct 2, 2019
Contributor
the !== false here makes it hard for my brain to read this
the !== false here makes it hard for my brain to read this
rwjblue
Oct 2, 2019
Author
Member
kk, ya I can fix that up
kk, ya I can fix that up
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Generally speaking, ember-cli allows each "addon layer" in the app that is transpiling its own templates. Each layer is supposed to be able to provide its own AST plugins without affecting the others. Unfortunately, the implementation of
Ember.HTMLBars.registerPluginmutates a single shared module scope array to track which plugins should be ran.In order to allow the layered approach mentioned above, ember-cli-htmlbars was forced to invalidate the
require.cacheanddeletevarious things off of theglobalobject. Requiringember-template-compiler.js(~1.2mb) many times (once for the project and again for each addon that depends onember-cli-htmlbars) is a non-trivial percentage of every app's build time and overall memory.This PR begins the process of removing that manual cache invalidation, in favor of better APIs provided by Ember (as of Ember 1.13 and higher).
In order to stop the manual cache invalidation but still provide the guarantees that each layer's AST plugins do not affect the others' we must still continue to invalidate the caches cache busting system if we detect that anyone in the system is using the global state mutation.
Specific changes in this PR:
Ember.HTMLBars.registerPlugin), instead pass the plugins directly toEmber.HTMLBars.precompilefor each module.ember-cli-htmlbarsorember-cli-htmlbars-inline-precompilethat still uses global state mutation.require.cachewhen the requirements are met.TODOs:
Future PR TODO: