Support circular dependencies and export let with experimentalImportSupport#1429
Open
Support circular dependencies and export let with experimentalImportSupport#1429
Conversation
facebook-github-bot
added
the
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D68394514 |
facebook-github-bot
pushed a commit
that referenced
this pull request
Jan 20, 2025
Summary:
Currently, `experimentalImportSupport` transforms named imports such that they're accessed immediately at the top level, eg:
```js
import {foo} from 'bar';
export function getFoo() {
return foo;
}
```
Becomes
```js
Object.defineProperty(exports, '__esModule', {
value: true
});
var foo = require('bar').foo;
function getFoo() {
return foo;
}
exports.getFoo = getFoo;
```
This immediate, top-level assignment of `require('bar').foo` to `foo` problematic for two reasons:
1. In the case of circular dependencies, the module at `'bar'` may not have been fully initialised, so that `foo` might be undefined at this point.
2. In the case where `bar` defines `export let foo = 'something mutable'`, a reassigment of `foo` within `'bar'` at runtime will not be reflected by the importing module.
Differential Revision: D68394514
facebook-github-bot
force-pushed
the
export-D68394514
branch
from
f38e4e3 to
871238f
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D68394514 |
Summary:
NOTE: This is an opt-in extension to `experimentalImportSupport`, this diff is a no-op by default.
Currently, `experimentalImportSupport` transforms named imports such that they're accessed immediately at the top level, eg:
```js
import {foo} from 'bar';
export function getFoo() {
return foo;
}
```
Becomes
```js
Object.defineProperty(exports, '__esModule', {
value: true
});
var foo = require('bar').foo;
function getFoo() {
return foo;
}
exports.getFoo = getFoo;
```
This immediate, top-level assignment of `require('bar').foo` to `foo` problematic for two reasons:
1. In the case of circular dependencies, the module at `'bar'` may not have been fully initialised, so that `foo` might be undefined at this point.
2. In the case where `bar` defines `export let foo = 'something mutable'`, a reassignment of `foo` within `'bar'` at runtime will not be reflected by the importing module.
This aims to fix 1 and get closer to a fix for 2. The new output would be:
```js
Object.defineProperty(exports, '__esModule', {
value: true
});
var _bar = require('bar');
function getFoo() {
return _bar.foo;
}
exports.getFoo = getFoo;
```
By lazily accessing values
Differential Revision: D68394514
facebook-github-bot
force-pushed
the
export-D68394514
branch
from
871238f to
4ebd33b
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D68394514 |
|
Hey @robhogan, I wanted to say thanks for this fix and ask if anything is blocking this from merging. My team wants to try the React Compiler on our Expo project, and this is the current blocker for that. Thanks again! |
|
Hey just wondering if there's any update? With storybook we're seeing issues with enabling the react compiler that seem like they could be related to this. |
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
4 participants
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:
Currently,
experimentalImportSupporttransforms named imports such that they're accessed immediately at the top level, eg:Becomes
This immediate, top-level assignment of
require('bar').footofoois problematic for two reasons:'bar'may not have been fully initialised, so thatfoomight be undefined at this point.bardefinesexport let foo = 'something mutable', a reassigment offoowithin'bar'at runtime will not be reflected by the importing module.Differential Revision: D68394514