Skip to content

Migrate to ESM import/export syntax#1549

Closed
robhogan wants to merge 15 commits into
mainfrom
export-D79818774
Closed

Migrate to ESM import/export syntax#1549
robhogan wants to merge 15 commits into
mainfrom
export-D79818774

Conversation

@robhogan

@robhogan robhogan commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

Migrate Metro to ESM syntax so that we can set up automatic generation of TypeScript types.

Differential Revision: D79818774

@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 Aug 7, 2025
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 7, 2025
Summary:
Pull Request resolved: #1549

Rollback Plan:

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary:
Pull Request resolved: #1549

Rollback Plan:

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary:
Pull Request resolved: #1549

Rollback Plan:

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary:
Pull Request resolved: #1549

Rollback Plan:

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary: Pull Request resolved: #1549

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary: Pull Request resolved: #1549

Differential Revision: D79818774
@robhogan robhogan changed the title Enable import/no-commonjs lint Migrate to ESM import/export syntax Aug 8, 2025
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 8, 2025
Summary:
Completes the migration of Metro to ESM syntax by enabling the `import/no-commonjs` lint (error unless suppressed) across all source files that run under Node.js (ie, not `metro-runtime`).

Remove the temporary `lint/no-commonjs-requires`, which covers a subset of `import/no-commonjs`.

Changelog: Internal

Pull Request resolved: #1549

Differential Revision: D79818774
Summary:
We want to convert Metro to ESM syntax in order to make better use of Flow's API translator (and for modernisation and consistency).

The first step will be to rewrite `require` calls as static `import`s. This adds a temporary linter to help us.

Why temporary? [`import/no-commonjs`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-commonjs.md) is the OSS standard for this purpose, but unlike the rule in this diff it has no auto-fix, which will be useful. `import/no-commonjs` also doesn't allow us to lint against `require` while allowing `module.exports`.

Why commit this at all? The rule is deleted at the end of this stack in favour of `import/commonjs`, but we'll need it throughout the stack for test plans. And, we may want to restore it later for other parts of the codebase.

Differential Revision: D79816826
Summary:
## Stack
In the first phase of migrating Metro to use consistent ESM syntax, we'll replace `require` calls with `import` expressions wherever possible.

We're starting with `require` calls so that we go through  `interopRequireDefault` on default imports, which will subsequently allow us to gradually migrate `module.exports = ` to `export default` (under Babel's `babel` interop mode) without breaking the importing side.

## This diff
Replaces `require` calls in the main (and largest) `metro` package.

Changelog: Internal

Differential Revision: D79728327
Summary:
## Stack
In the first phase of migrating Metro to use consistent ESM syntax, we'll replace `require` calls with `import` expressions wherever possible.

We're starting with `require` calls so that we go through  `interopRequireDefault` on default imports, which will subsequently allow us to gradually migrate `module.exports = ` to `export default` (under Babel's `babel` interop mode) without breaking the importing side.

## This diff
Replaces `require` calls in `metro-file-map`, apart from dependencies of the `worker.js` file, because it runs directly from source without Babel registration.

Changelog: Internal

Differential Revision: D79731832
Summary:
## Stack
In the first phase of migrating Metro to use consistent ESM syntax, we'll replace `require` calls with `import` expressions wherever possible.

We're starting with `require` calls so that we go through  `interopRequireDefault` on default imports, which will subsequently allow us to gradually migrate `module.exports = ` to `export default` (under Babel's `babel` interop mode) without breaking the importing side.

## This diff
Replaces `require` calls in `metro-resolver`.

Changelog: Internal

Differential Revision: D79732102
Summary:
## Stack
In the first phase of migrating Metro to use consistent ESM syntax, we'll replace `require` calls with `import` expressions wherever possible.

We're starting with `require` calls so that we go through  `interopRequireDefault` on default imports, which will subsequently allow us to gradually migrate `module.exports = ` to `export default` (under Babel's `babel` interop mode) without breaking the importing side.

## This diff
Replaces `require` calls or marks them specifically ignored by the linter in all remaining packages.

Changelog: Internal

Differential Revision: D79732543
Differential Revision: D79735956
Differential Revision: D79741120
Differential Revision: D79799084
Differential Revision: D79804092
Differential Revision: D79804094
Differential Revision: D79804093
Differential Revision: D79804098
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

robhogan added a commit that referenced this pull request Aug 11, 2025
Summary:
Completes the migration of Metro to ESM syntax by enabling the `import/no-commonjs` lint (error unless suppressed) across all source files that run under Node.js (ie, not `metro-runtime`) and run through Babel when running from source (ie, not `metro-babel-register`).

Remove the temporary `lint/no-commonjs-requires`, which covers a subset of `import/no-commonjs`.

Changelog: Internal

Pull Request resolved: #1549

Reviewed By: vzaidman

Differential Revision: D79818774
Summary:
Completes the migration of Metro to ESM syntax by enabling the `import/no-commonjs` lint (error unless suppressed) across all source files that run under Node.js (ie, not `metro-runtime`) and run through Babel when running from source (ie, not `metro-babel-register`).

Remove the temporary `lint/no-commonjs-requires`, which covers a subset of `import/no-commonjs`.

Changelog: Internal

Pull Request resolved: #1549

Reviewed By: vzaidman

Differential Revision: D79818774
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D79818774

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been merged in 199a990.

facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2025
… 'metro-pkg'`) in Metro public Node.js APIs.

Summary:
In #1549 (Migrate Metro to ESM syntax), which was intended to be non-breaking, there was one use case I overlooked.

If your code consuming Metro under Node.js uses ESM-style imports transformed to CommonJS with Babel's default interop mode, you could've written any of:

```
// Import default
import Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or,


```
// Import all named as namespace
import * as Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or, 


```
// Import named
import {resolve} from 'metro-resolver';
resolve('./foo');
```

However, when we switched from a `module.exports` object to ESM-style named exports, the first case would fail, because Babel marks our index file with `__esModule`, which causes `interopRequireDefault` to select the `default` prop (which is now undefined).

The consumer would be transformed to this, which would throw:

```
var _metroResolver = _interopRequireDefault(require("metro-resolver"));
function _interopRequireDefault(e) { return e && e.__esModule ? e : { default: e }; }
_metroResolver.default.resolve('./foo'); // TypeError: Cannot access property resolve of undefined
```

This restores compatibility by adding an explicit default export alongside the named exports, for all public package entry points that were not already `__esModule`s. These will be removed in a future breaking change.

Changelog: Internal - fix unreleased breaking change from #1549

Reviewed By: vzaidman

Differential Revision: D81127738
facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2025
… 'metro-pkg'`) in Metro public Node.js APIs. (#1565)

Summary:

In #1549 (Migrate Metro to ESM syntax), which was intended to be non-breaking, there was one use case I overlooked.

If your code consuming Metro under Node.js uses ESM-style imports transformed to CommonJS with Babel's default interop mode, you could've written any of:

```
// Import default
import Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or,


```
// Import all named as namespace
import * as Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or, 


```
// Import named
import {resolve} from 'metro-resolver';
resolve('./foo');
```

However, when we switched from a `module.exports` object to ESM-style named exports, the first case would fail, because Babel marks our index file with `__esModule`, which causes `interopRequireDefault` to select the `default` prop (which is now undefined).

The consumer would be transformed to this, which would throw:

```
var _metroResolver = _interopRequireDefault(require("metro-resolver"));
function _interopRequireDefault(e) { return e && e.__esModule ? e : { default: e }; }
_metroResolver.default.resolve('./foo'); // TypeError: Cannot access property resolve of undefined
```

This restores compatibility by adding an explicit default export alongside the named exports, for all public package entry points that were not already `__esModule`s. These will be removed in a future breaking change.

Changelog: Internal - fix unreleased breaking change from #1549

Reviewed By: vzaidman

Differential Revision: D81127738
facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2025
… 'metro-pkg'`) in Metro public Node.js APIs. (#1565)

Summary:

In #1549 (Migrate Metro to ESM syntax), which was intended to be non-breaking, there was one use case I overlooked.

If your code consuming Metro under Node.js uses ESM-style imports transformed to CommonJS with Babel's default interop mode, you could've written any of:

```
// Import default
import Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or,


```
// Import all named as namespace
import * as Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or, 


```
// Import named
import {resolve} from 'metro-resolver';
resolve('./foo');
```

However, when we switched from a `module.exports` object to ESM-style named exports, the first case would fail, because Babel marks our index file with `__esModule`, which causes `interopRequireDefault` to select the `default` prop (which is now undefined).

The consumer would be transformed to this, which would throw:

```
var _metroResolver = _interopRequireDefault(require("metro-resolver"));
function _interopRequireDefault(e) { return e && e.__esModule ? e : { default: e }; }
_metroResolver.default.resolve('./foo'); // TypeError: Cannot access property resolve of undefined
```

This restores compatibility by adding an explicit default export alongside the named exports, for all public package entry points that were not already `__esModule`s. These will be removed in a future breaking change.

Changelog: Internal - fix unreleased breaking change from #1549

Reviewed By: vzaidman

Differential Revision: D81127738
facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2025
… 'metro-pkg'`) in Metro public Node.js APIs. (#1565)

Summary:
In #1549 (Migrate Metro to ESM syntax), which was intended to be non-breaking, there was one use case I overlooked.

If your code consuming Metro under Node.js uses ESM-style imports transformed to CommonJS with Babel's default interop mode, you could've written any of:

```
// Import default
import Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or,

```
// Import all named as namespace
import * as Resolver from 'metro-resolver';
Resolver.resolve('./foo');
```

or,

```
// Import named
import {resolve} from 'metro-resolver';
resolve('./foo');
```

However, when we switched from a `module.exports` object to ESM-style named exports, the first case would fail, because Babel marks our index file with `__esModule`, which causes `interopRequireDefault` to select the `default` prop (which is now undefined).

The consumer would be transformed to this, which would throw:

```
var _metroResolver = _interopRequireDefault(require("metro-resolver"));
function _interopRequireDefault(e) { return e && e.__esModule ? e : { default: e }; }
_metroResolver.default.resolve('./foo'); // TypeError: Cannot access property resolve of undefined
```

This restores compatibility by adding an explicit default export alongside the named exports, for all public package entry points that were not already `__esModule`s. These will be removed in a future breaking change.

Changelog: Internal - fix unreleased breaking change from #1549

Test Plan:
Imported from GitHub, without a `Test Plan:` line.

Rollback Plan:

Differential Revision: D81127738

Pulled By: robhogan

fbshipit-source-id: c4caf30cc75ec7f7f3a87ed8df206c0fa0c58ffd
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. fb-exported Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants