Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Babel-Loader Caching for ember-template-compiler #839

Merged
merged 1 commit into from Jun 9, 2021

Conversation

stefanpenner
Copy link
Collaborator

@stefanpenner stefanpenner commented Jun 9, 2021

Fix Babel-Loader Caching for ember-template-compiler

Today, when switching template-compiler versions, we can end up with global cache pollution. Although this can manifest many ways, it most commonly manifests when running ember-try across versions of ember with different template compilation semantics when also using inline templates.

To address this:

We now include the checksum of the template compiler in the inline-template-compiler babel configuration options.
This busts the cache, as any functioning caching babel subsystem (such as babel-loader) must consider option changes as cache invalidating scenarios. Today, it is in-fact the case that babel-loader operates as we expect, and with this change, caches are correctly invalidated.

We co-located the checksum with the template-compiler options, rather then a single global cache key to both simplify debugging and not prevent some more granular caching strategy from functioning optimally.

This does not explicitly include integration tests due to my believe that this is already covered sufficiently:

  1. today’s tests already include this (running yarn test currently fails without this fix)
  2. type checking ensure we now always pass the key in
  3. this is ultimately a configuration change of a babel-loader feature (which itself should test this)

@stefanpenner
Copy link
Collaborator Author

I indent to follow this up with (3) more changes:

  1. include additional cache busting configuration changes for portable babel loader options (node_module version)
  2. include additional cache busting configuration changes for each babel plugin (plugins version)
  3. readme addition describing how this caching works, and when & how a power-user may need to bust the cache.

@rwjblue rwjblue added the bug Something isn't working label Jun 9, 2021
@rwjblue rwjblue linked an issue Jun 9, 2021 that may be closed by this pull request
@stefanpenner stefanpenner force-pushed the cache-busting branch 3 times, most recently from b7dff7f to 9d637d8 Compare June 9, 2021 18:39
Today, when switching template-compiler versions, we can end up with global cache pollution. Although this can manifest many ways, it most commonly manifests when running ember-try across versions of ember with different template compilation semantics when also using inline templates.

To address this:

We now include the checksum of the template compiler in the inline-template-compiler babel configuration options.
This busts the cache, as any functioning caching babel subsystem (such as babel-loader) must consider option changes as cache invalidating scenarios. Today, it is in-fact the case that babel-loader operates as we expect, and with this change, caches are correctly invalidated.

We co-located the checksum with the template-compiler options, rather then a single global cache key to both simplify debugging and not prevent some more granular caching strategy from functioning optimally.

Finally this commit uses the existing caching infrastructure to load / generate cacheKey’s for the template-compiler. It changes that infrastructure slightly only to ensure a vm.script is create when absolutely needed.

This does not explicitly include integration tests due to my believe that this is already covered sufficiently:

1) today’s tests already include this (running yarn test currently fails without this fix)
2) type checking ensure we now always pass the key in
3) this is ultimately a configuration change of a babel-loader feature (which itself should test this)
@rwjblue rwjblue merged commit 9cd5f80 into master Jun 9, 2021
@rwjblue rwjblue deleted the cache-busting branch June 9, 2021 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue [Test Failures]
2 participants