This repository has been archived by the owner on Dec 8, 2024. It is now read-only.
Avoid sharing list of previously added imports. #355
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
…le files
```
TypeError: /Users/rjackson/src/ember-cli/babel-plugin-htmlbars-inline-precompile/foo-bar.js: C
annot read property 'reference' of undefined
14 | for (let ref of refPaths) {
15 | let binding = ref.scope.getBinding(ref.node.name);
> 16 | binding.reference(ref);
| ^
17 | }
18 | };
19 |
at reference (src/util.js:16:13)
at registerRefs (index.js:143:7)
at PluginPass.replacePath (index.js:383:7)
at newFn (node_modules/@babel/traverse/lib/visitors.js:175:21)
```
Simply guarding for `ref.scope.getBinding(...)` returning `undefined` is
not "good enough".
The types allow for returning `undefined`, but we always assumed it
would return a reference binding.
Note: this does not fix the underlying tests, they now fail to add the
module imports for `createTemplateFactory`:
```
FAIL __tests__/tests.js
● htmlbars-inline-precompile › does not error when transpiling multiple modules with a single pl
ugin config
expect(received).toMatchInlineSnapshot(snapshot)
Snapshot name: `htmlbars-inline-precompile does not error when transpiling multiple modules wi
th a single plugin config 2`
- Snapshot - 2
+ Received + 0
- import { createTemplateFactory as _createTemplateFactory } from "@ember/template-factory";
-
var compiled = _createTemplateFactory(
/*
hello
*/
"precompiled(hello)");
84 | );
85 |
> 86 | expect(transpiled).toMatchInlineSnapshot(`
| ^
87 | "import { createTemplateFactory as _createTemplateFactory } from \\"@ember/templa
te-factory\\";
88 |
89 | var compiled = _createTemplateFactory(
at Object.toMatchInlineSnapshot (__tests__/tests.js:86:24)
```
Creating `allAddedImports` in plugin scope "bleeds" across repeated transpilations. This babel plugin function is invoked once for each configuration of babel plugins, **not** once per transpilation attempt. This moves `allAddedImports` onto `state` so that it is reset between transpilation attempts.
rwjblue
force-pushed
the
avoid-shared-mutable-state
branch
from
8747af0 to
8a5738c
Compare
pzuraq
approved these changes
Mar 12, 2021
This was referenced Mar 15, 2021
Closed
Closed
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.
The test added here shows the underlying problem, when transpiling multiple files with a shared babel configuration (which is very normal), you would end up with failures if any transformation fails (all future transformations would continue to fail):
The fundamental issue here is that we shared the list of previously imported modules so that we can make sure their import bindings have been rewritten to use the babel-plugin-ember-modules-api-polyfill transformation. Unfortunately, we tracked that state in the closure state of the babel plugin. That babel plugin closure state is only evaluated / created once not once per-file.
This changes things to track the module imports that have been created on the local state (which is reset properly per "transpilation attempt").