Skip to content

Commit

Permalink
import-export-plugin: Rename potentially conflicting declarations at …
Browse files Browse the repository at this point in the history
…module scope (module, exports, require, global)

Summary:
Currently, when using `experimentalImportPlugin` (`import-export-plugin`) without `babel-plugin-transform-module-commonjs`, the following completely legal ESM has broken exports and broken internal behaviour:

```
const exports = {}
export const foo = 'bar';
process.nextTick(() => {
  console.log(exports); // should print "{}"
});

```

Because it transforms to this, where `var exports` does not throw (as `const` or `let` would for an already-bound name) but it does shadow the injected `exports` argument, so that no assignment to the injected `exports` is ever made (in other words, a consumer would see no defined exports). In fact, an assignment to the local `exports` *is* made where it shouldn't be.

```
__d(function (global, require, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _$$_METRO_DEPENDENCY_MAP) {
  "use strict";

  Object.defineProperty(exports, '__esModule', {
    value: true
  });
  var exports = {};
  var baz = 'foo';
  process.nextTick(() => {
    console.log(exports); // Now prints "{baz:  'foo'}" 
  });
  exports.baz = baz;
},/*...*/);
```

Moreover, in a configuration where we don't transform `const`/`let` to `var` (eg targeting web or server), this actually throws at runtime, as would binding any of `module`, `global` or `require`.

The solution is to rename bindings to names that would be fine in ESM but conflict with CommonJS names / Metro's module factory

The analogue in the (roughly) corresponding Babel plugin is here: https://github.com/babel/babel/blob/38d26cd5eeb66b697671cfb8c78f963f02992073/packages/babel-plugin-transform-modules-commonjs/src/index.ts#L198-L204

(Note that we do not inject `__dirname` or `__filename` in Metro, but we do inject `global`)

Changelog:
```
**[Fix]**: import-exports-plugin: Rename `module`/`require`/`exports`/`global` declarations to avoid conflicts with injected args or generated code.
```

Differential Revision: D65536715
  • Loading branch information
robhogan authored and facebook-github-bot committed Nov 9, 2024
1 parent c5a4a9a commit caa9d90
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,61 @@ test('exports named members', () => {
compare([importExportPlugin], code, expected, opts);
});

test('renames existing `exports` declarations in module scope', () => {
const code = `
const exports = 'foo';
export const bar = 'bar';
console.log(exports, bar);
`;

const expected = `
Object.defineProperty(exports, '__esModule', {value: true});
const _exports = 'foo';
const bar = 'bar';
console.log(_exports, bar);
exports.bar = bar;
`;

compare([importExportPlugin], code, expected, opts);
});

test('handles an export named "exports"', () => {
const code = `
export const exports = {a: 'foo'};
`;

const expected = `
Object.defineProperty(exports, '__esModule', {value: true});
const _exports = {
a: 'foo',
};
exports.exports = _exports;
`;

compare([importExportPlugin], code, expected, opts);
});

test('allows mixed esm and cjs exports', () => {
const code = `
export const foo = 'foo';
exports.bar = 'bar';
module.exports.baz = 'baz';
export default class {}
`;

const expected = `
Object.defineProperty(exports, '__esModule', {value: true});
const foo = 'foo';
exports.bar = 'bar';
module.exports.baz = 'baz';
class _default {}
exports.default = _default;
exports.foo = foo;
`;

compare([importExportPlugin], code, expected, opts);
});

test('exports destructured named object members', () => {
const code = `
export const {foo,bar} = {foo: 'bar',bar: 'baz'};
Expand Down Expand Up @@ -266,6 +321,24 @@ test('enables module exporting when something is exported', () => {
`);
});

test('renames bindings', () => {
const code = `
const module = 'foo';
let exports = 'bar';
var global = 'baz';
const require = {};
`;

const expected = `
const _module = 'foo';
let _exports = 'bar';
var _global = 'baz';
const _require = {};
`;

compare([importExportPlugin], code, expected, opts);
});

test('supports `import {default as LocalName}`', () => {
const code = `
import {
Expand Down
8 changes: 8 additions & 0 deletions packages/metro-transform-plugins/src/import-export-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,14 @@ function importExportPlugin({types: t}: {types: Types, ...}): PluginObj<State> {
state.imports = [];
state.importAll = t.identifier(state.opts.importAll);
state.importDefault = t.identifier(state.opts.importDefault);

// Rename declarations at module scope that might otherwise conflict
// with arguments we inject into the module factory.
// Note that it isn't necessary to rename importAll/importDefault
// because Metro already uses generateUid to generate unused names.
['module', 'global', 'exports', 'require'].forEach(name =>
path.scope.rename(name),
);
},

exit(path: NodePath<Program>, state: State): void {
Expand Down

0 comments on commit caa9d90

Please sign in to comment.