Skip to content

Commit

Permalink
Support circular dependencies and export let with experimentalImportS…
Browse files Browse the repository at this point in the history
…upport

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
  • Loading branch information
robhogan authored and facebook-github-bot committed Jan 19, 2025
1 parent ab33059 commit f38e4e3
Show file tree
Hide file tree
Showing 8 changed files with 316 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/metro-babel-transformer/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type BabelTransformerOptions = $ReadOnly<{
enableBabelRCLookup?: boolean,
enableBabelRuntime: boolean | string,
extendsBabelConfigPath?: string,
experimentalImportSupport?: boolean,
experimentalImportSupport?: boolean | $ReadOnly<{importAsObjects?: boolean}>,
hermesParser?: boolean,
hot: boolean,
minify: boolean,
Expand Down
4 changes: 3 additions & 1 deletion packages/metro-babel-transformer/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export interface BabelTransformerOptions {
readonly enableBabelRCLookup?: boolean;
readonly enableBabelRuntime: boolean | string;
readonly extendsBabelConfigPath?: string;
readonly experimentalImportSupport?: boolean;
readonly experimentalImportSupport?:
| boolean
| Readonly<{importAsObjects?: boolean}>;
readonly hermesParser?: boolean;
readonly hot: boolean;
readonly minify: boolean;
Expand Down
6 changes: 5 additions & 1 deletion packages/metro-config/src/configTypes.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ export type ExtraTransformOptions = $ReadOnly<{
preloadedModules?: $ReadOnly<{[path: string]: true, ...}> | false,
ramGroups?: $ReadOnlyArray<string>,
transform?: $ReadOnly<{
experimentalImportSupport?: boolean,
experimentalImportSupport?:
| boolean
| $ReadOnly<{
importAsObjects?: boolean,
}>,
inlineRequires?:
| $ReadOnly<{blockList: $ReadOnly<{[string]: true, ...}>, ...}>
| boolean,
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-config/types/configTypes.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface ExtraTransformOptions {
readonly preloadedModules: Readonly<{[path: string]: true}> | false;
readonly ramGroups: ReadonlyArray<string>;
readonly transform: Readonly<{
experimentalImportSupport: boolean;
experimentalImportSupport: boolean | Readonly<{importAsObjects?: boolean}>;
inlineRequires:
| Readonly<{blockList: Readonly<{[path: string]: true}>}>
| boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

import type {Options} from '../import-export-plugin';
import type {Dependency} from 'metro/src/ModuleGraph/worker/collectDependencies';

const {compare, transformToAst} = require('../__mocks__/test-helpers');
Expand All @@ -19,9 +20,10 @@ const importExportPlugin = require('../import-export-plugin');
const {codeFrameColumns} = require('@babel/code-frame');
const collectDependencies = require('metro/src/ModuleGraph/worker/collectDependencies');

const opts = {
const opts: Options = {
importAll: '_$$_IMPORT_ALL',
importDefault: '_$$_IMPORT_DEFAULT',
resolve: false,
};

test('correctly transforms and extracts "import" statements', () => {
Expand Down Expand Up @@ -375,6 +377,141 @@ test('supports `import {default as LocalName}`', () => {
`);
});

describe('importAsObjects == true', () => {
const innerOpts: Options = {...opts, importAsObjects: true};
test('named import refs transform to object property access', () => {
const code = `
import {'foo-bar' as fooBar, baz} from 'qux';
const obj = {fooBar, baz};
console.log(fooBar, baz);
`;
const expected = `
var _qux = require('qux');
const obj = {
fooBar: _qux["foo-bar"],
baz: _qux.baz
};
console.log(_qux["foo-bar"], _qux.baz);
`;
compare([importExportPlugin], code, expected, innerOpts);
});

test('re-exporting named imports', () => {
const code = `
import {'foo-bar' as fooBar, baz} from 'qux';
export {fooBar, baz as myBaz};
`;
const expected = `
Object.defineProperty(exports, '__esModule', {
value: true
});
var _qux = require('qux');
exports.fooBar = _qux["foo-bar"];
exports.myBaz = _qux.baz;
`;
compare([importExportPlugin], code, expected, innerOpts);
});

test('namespace import refs maintain property access', () => {
const code = `
import * as Foo from 'foo';
console.log(Foo.bar, Foo.baz);
`;
const expected = `
var Foo = _$$_IMPORT_ALL('foo');
console.log(Foo.bar, Foo.baz);
`;
compare([importExportPlugin], code, expected, innerOpts);
});

test('mixed default, default-by-name and named imports`', () => {
const code = `
import RN, {
View,
Platform as RNPlatform,
default as ReactNative
} from 'react-native';
import ReactNative2 from 'react-native';
console.log(View, RNPlatform, ReactNative, RN);
function myFunc() {
const RNPlatform = 'foo';
RN.bar();
React.foo();
return RNPlatform;
}
function myOtherFunc() {
return RNPlatform;
}
`;

const expected = `
var _reactNative = require('react-native');
var RN = _$$_IMPORT_DEFAULT('react-native');
var ReactNative = RN;
var ReactNative2 = _$$_IMPORT_DEFAULT('react-native');
console.log(_reactNative.View, _reactNative.Platform, ReactNative, RN);
function myFunc() {
const RNPlatform = 'foo';
RN.bar();
React.foo();
return RNPlatform;
}
function myOtherFunc() {
return _reactNative.Platform;
}
`;

compare([importExportPlugin], code, expected, innerOpts);

// Expect three dependencies mapping to the first import declaration...
// 1. RN (import default)
// 2. View and Platform (import named)
// 3. ReactNative (import default as)
// ...and one mapping to the second import declaration
// 4. ReactNative2 (import default)
// All should share the same dependency key (at 0) because all are resolved
// identically.
expect(showTransformedDeps(code)).toMatchInlineSnapshot(`
"
> 2 | import RN, {
| ^^^^^^^^^^^^
> 3 | View,
| ^^^^^^^^^^^
> 4 | Platform as RNPlatform,
| ^^^^^^^^^^^
> 5 | default as ReactNative
| ^^^^^^^^^^^
> 6 | } from 'react-native';
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #0 (react-native)
> 2 | import RN, {
| ^^^^^^^^^^^^
> 3 | View,
| ^^^^^^^^^^^
> 4 | Platform as RNPlatform,
| ^^^^^^^^^^^
> 5 | default as ReactNative
| ^^^^^^^^^^^
> 6 | } from 'react-native';
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #0 (react-native)
> 2 | import RN, {
| ^^^^^^^^^^^^
> 3 | View,
| ^^^^^^^^^^^
> 4 | Platform as RNPlatform,
| ^^^^^^^^^^^
> 5 | default as ReactNative
| ^^^^^^^^^^^
> 6 | } from 'react-native';
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #0 (react-native)
> 7 | import ReactNative2 from 'react-native';
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dep #0 (react-native)"
`);
});
});

function showTransformedDeps(code: string) {
const {dependencies} = collectDependencies(
transformToAst([importExportPlugin], code, opts),
Expand Down
Loading

0 comments on commit f38e4e3

Please sign in to comment.