experimentalImportSupport: Fix exported names under relabelling and rest spread (#1772)#1772
Closed
robhogan wants to merge 1 commit into
Closed
experimentalImportSupport: Fix exported names under relabelling and rest spread (#1772)#1772robhogan wants to merge 1 commit into
robhogan wants to merge 1 commit into
Conversation
Contributor
|
@robhogan has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111013897. |
…est spread (#1772) Summary: Fixes #1460 Fixes two related bugs in Metro's `experimentalImportSupport`. ## Exports with inline relabelling ```js export const { A: B } = {}; ``` **Currently transforms to** ```js const { A: B } = {}; exports.A = A; ``` Which is invalid: `A` is a reference error. **After this diff** ```js const { A: B } = {}; exports.A = B; ``` ## Exports with spreads Currently both of these fail to transform: **Object spread** ```js export const {A, ...others} = {A: 'A', B: 'B', C: 'C'}; ``` Plugin throws `TypeError: Cannot read properties of undefined (reading 'name')` when attempting to iterate over the object for key: val pairs. **Array spread** ```js export const [A, ...others] = ['A', 'B', 'C']; ``` Plugin throws `Property name expected type of string but got undefined` after extracting an `undefined` name and then attempting to use it at as prop of `exports`. **After this diff** ```js const {A, ...others} = {A: 'A', B: 'B', C: 'C'}; exports.A = A; exports.others = others; ``` and ```js const [A, ...others] = ['A', 'B', 'C']; exports.A = A; exports.others = others; ``` ## Spec Almost goes without saying in this case - exported variable declarations should expose the declaration's bound names. ECMA-262 defines those names through Static Semantics: `BoundNames`, including `BindingPattern` forms: https://tc39.es/ecma262/#sec-static-semantics-boundnames ## Implementation Metro had naive `ObjectPattern`/`ArrayPattern` extraction that read object keys or array elements directly. This fixes the same issues Expo did in [#38058](expo/expo#38058) and [#38118](expo/expo#38118), but uses Babel's own `getBindingIdentifiers` for a simpler implementation. ## Changelog ``` - **[Experimental]**: experimentalImportSupport: Fix exports renamed by destructuring, and support exports of rest spreads. ``` Reviewed By: GijsWeterings, huntie Differential Revision: D111013897
455bb9e to
d0d73f6
Compare
Contributor
|
This pull request has been merged in 0dc5030. |
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:
Fixes #1460
Fixes two related bugs in Metro's
experimentalImportSupport.Exports with inline relabelling
Currently transforms to
Which is invalid:
Ais a reference error.After this diff
Exports with spreads
Currently both of these fail to transform:
Object spread
Plugin throws
TypeError: Cannot read properties of undefined (reading 'name')when attempting to iterate over the object for key: val pairs.Array spread
Plugin throws
Property name expected type of string but got undefinedafter extracting anundefinedname and then attempting to use it at as prop ofexports.After this diff
and
Spec
Almost goes without saying in this case - exported variable declarations should expose the declaration's bound names. ECMA-262 defines those names through Static Semantics:
BoundNames, includingBindingPatternforms: https://tc39.es/ecma262/#sec-static-semantics-boundnamesImplementation
Metro had naive
ObjectPattern/ArrayPatternextraction that read object keys or array elements directly.This fixes the same issues Expo did in #38058 and #38118, but uses Babel's own
getBindingIdentifiersfor a simpler implementation.Changelog
Reviewed By: GijsWeterings, huntie
Differential Revision: D111013897