import-export-plugin: Rename potentially conflicting declarations at module scope (module, exports, require, global)

robhogan commented 1 week ago

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 foo = 'bar';
  process.nextTick(() => {
    console.log(exports); // Now prints "{foo:  'bar'}" 
  });
  exports.foo = foo;
},/*...*/);

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