experimentalImportSupport: Place export-all assignments before explicit exports, fix precedence of export default > export * from#1773
Closed
robhogan wants to merge 1 commit into
Closed
Conversation
…it exports, fix precedence of `export default` > `export * from`
Summary:
Fixes a bug in `experimentalImportSupport` where `export * from 'foo'` would incorrectly overwrite an `export default`, if `'foo'` also has a default export.
# Example
## Input source
```js
// foo.js (the star source)
export default 'star default';
export const overridden = 'star named';
export const sourceOnly = 'source only';
```
```js
// entry.js
export * from 'foo';
export const overridden = 'explicit named';
export default 'explicit default';
```
Per the ES spec, importing from `entry.js` should give `overridden === 'explicit named'`, `default === 'explicit default'`, and `sourceOnly === 'source only'`. Explicit exports take precedence over names supplied via `export *`, regardless of the position of export declarations.
## Before
```js
'use strict';
Object.defineProperty(exports, '__esModule', {value: true});
var _overridden = 'explicit named';
var _default = 'explicit default';
exports.overridden = _overridden; // 'explicit named'
exports.default = _default; // 'explicit default'
var _foo = require('foo');
for (var _key in _foo) { // star copy runs LAST...
exports[_key] = _foo[_key]; // ...clobbers the explicit exports
}
```
Runtime result (wrong):
```js
exports.overridden // 'star named' ❌ should be 'explicit named'
exports.default // 'star default' ❌ should be 'explicit default'
exports.sourceOnly // 'source only' ✅
```
## After
```js
'use strict';
Object.defineProperty(exports, '__esModule', {value: true});
var _overridden = 'explicit named';
var _default = 'explicit default';
var _foo = require('foo');
for (var _key in _foo) { // star copy runs FIRST
exports[_key] = _foo[_key];
}
exports.overridden = _overridden; // explicit exports now win
exports.default = _default;
```
Runtime result (spec-correct):
```js
exports.overridden // 'explicit named' ✅
exports.default // 'explicit default' ✅
exports.sourceOnly // 'source only' ✅
```
# Spec
[ECMA-262 Source Text Module Record](https://tc39.es/ecma262/#sec-source-text-module-records-resolveexport-exportname-resolve-set) `ResolveExport` checks local `ExportEntries` and indirect `ExportEntries` before considering `StarExportEntries`, so explicit exports take precedence over names supplied through `export *`.
# Provenance
This is part of the change made in Expo's fork in [#38111](expo/expo#38111) (though that's mainly concerned with introducing live bindings), so this is "in the wild" already for Expo users.
# Implementation
This is just a straightforward ordering swap of the emitted assignments.
# Changelog
```
- **[Experimental]**: `experimentalImportSupport` - fix precedence of `export default` over `export * from`.
```
Reviewed By: huntie
Differential Revision: D111013892
Contributor
|
@robhogan has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111013892. |
Contributor
|
This pull request has been merged in e6b154f. |
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 a bug in
experimentalImportSupportwhereexport * from 'foo'would incorrectly overwrite anexport default, if'foo'also has a default export.Example
Input source
Per the ES spec, importing from
entry.jsshould giveoverridden === 'explicit named',default === 'explicit default', andsourceOnly === 'source only'. Explicit exports take precedence over names supplied viaexport *, regardless of the position of export declarations.Before
Runtime result (wrong):
After
Runtime result (spec-correct):
Spec
ECMA-262 Source Text Module Record
ResolveExportchecks localExportEntriesand indirectExportEntriesbefore consideringStarExportEntries, so explicit exports take precedence over names supplied throughexport *.Provenance
This is part of the change made in Expo's fork in #38111 (though that's mainly concerned with introducing live bindings), so this is "in the wild" already for Expo users.
Implementation
This is just a straightforward ordering swap of the emitted assignments.
Changelog
Reviewed By: huntie
Differential Revision: D111013892