Repository navigation
Conversation
With `memoizeCalls`, destructured requires sorted their references by source position to decide which one assigns the memoized module. Nodes inserted by earlier plugins have no position, so an `exports.a = a` added by the import-export plugin could take the assignment while an earlier `a()` read the unassigned module. References are now sorted in program order, as `scope.crawl()` collects them.
Both the identifier and destructured paths swallowed `replaceWith` errors, eg for references in type annotations or `export {...}` specifiers, leaving a mix of inlined and original references. References are now checked up front with Babel's `validate`, and a require with any reference that can't be rewritten is left alone. The memo `var` is added to the program body directly, since `addStmtToBlock` can't fail for a `Program`.
Changelog: [Fix] Inline requires: Don't partially inline requires referenced in type annotations or `export {...}` specifiers
meta-cla
Bot
added
the
CLA Signed
This branch has not been deployed
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
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.
Summary
Follow-up to #2087 (unreleased), from @javache's review and a bug found along the way.
memoizeCalls, destructured references are now ordered by tree position, not source position. Nodes added by earlier plugins have no position, so the import-export plugin'sexports.a = awas taking the memo assignment:typeof fooorexport {foo}, are now checked up front with Babel'svalidate, and the require is left alone. Previously thereplaceWitherror was swallowed mid-rewrite, which withmemoizeCallsleft a dangling reference.const {} = require('m')keeps its require, for side effects.varis added to the program body directly -addStmtToBlockcouldn't fail for aProgram.Changelog: [Fix] Inline requires: Don't partially inline requires referenced in type annotations or
export {...}specifiersTest plan
New tests, which all fail before this change:
memoizeCallsexport {foo}specifiers, for both formsExisting snapshots are unchanged.