Skip to content

fix: Add ESM unwrapping to all CommonJS require calls#1675

Closed
kitten wants to merge 3 commits into
react:mainfrom
kitten:@kitten/fix/add-esm-unwrapping
Closed

fix: Add ESM unwrapping to all CommonJS require calls#1675
kitten wants to merge 3 commits into
react:mainfrom
kitten:@kitten/fix/add-esm-unwrapping

Conversation

@kitten

@kitten kitten commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

This was added to loadConfig.js but is missing in other places. While it's arguably not useful in many cases in a "standard" Metro setup with Metro's own packages, it'd be nice to have this behave consistently even with transpiled ESM output (or with require-ESM in Node).

Specifically this affects:

  • metro-file-map plugin workers that are loaded with require
  • metro-file-map's main worker loading
  • metro's asset plugins
  • metro-transform-worker's getMinifier implementation
  • metro-transform-worker's babelTransformerPath loading

This is probably specifically interesting for babelTransformerPath which is often overridden by third-party setups.

Changelog: [Fix] Unwrap ES modules when requiring metro-file-map workers, asset plugins, the minifier, or the babel transformer

Test plan

  • n/a: This will likely be best left untested, but is a consistent implementation that matches ESM->CJS semantics, also used in loadConfig.js

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 7, 2026
@kitten kitten force-pushed the @kitten/fix/add-esm-unwrapping branch from fa7ed4b to d831001 Compare April 11, 2026 13:00
@meta-codesync

meta-codesync Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

@robhogan has imported this pull request. If you are a Meta employee, you can view this in D102510513.

Comment on lines +80 to +81
const extractor =
mod.__esModule === true && 'default' in mod ? mod.default : mod;

@kitten kitten May 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted on a re-review that getCacheKey below means that we should be doing:

const getCacheKey =
  mod?.getCacheKey ??
  (mod.__esModule === true && 'default' in mod ? mod.default : mod).getCacheKey;

@meta-codesync

meta-codesync Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

@robhogan merged this pull request in 0e9d7aa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant