Skip to content

Support named exports with nodejs ESM#5162

Closed
robrichard wants to merge 1 commit intofacebook:mainfrom
robrichard:robrichard/cjs-named-exports
Closed

Support named exports with nodejs ESM#5162
robrichard wants to merge 1 commit intofacebook:mainfrom
robrichard:robrichard/cjs-named-exports

Conversation

@robrichard
Copy link
Copy Markdown
Contributor

It is currently not possible to use named exports with Relay in a Node.js ESM file. For example, in a brand new project with the latest version of relay-runtime as a dependency, the following code will error:

index.mjs:

import { fetchQuery } from 'relay-runtime'
file:///index.mjs:1
import { fetchQuery } from 'relay-runtime';
         ^^^^^^^^^^
SyntaxError: Named export 'fetchQuery' not found. The requested module 'relay-runtime' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'relay-runtime';
const { fetchQuery } = pkg;

    at #asyncInstantiate (node:internal/modules/esm/module_job:302:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:405:5)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:660:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:101:5)

When an ESM file imports a common js file (like relay-runtime) Node.js will use cjs-module-lexer to try to determine compatible named exports.

From the node.js docs:

Named exports detection covers many common export patterns, reexport patterns and build tool and transpiler outputs. See cjs-module-lexer for the exact semantics implemented.

In the entry points of react-relay and relay-runtime, the pattern of exporting a property on another object causes cjs-module-lexer to not detect any of named exports.

Example:

// This doesn't work
module.exports = {
  fetchQuery: RelayRuntime.fetchQuery
};

// This does work
const { fetchQuery } = RelayRuntime;
module.exports = {
  fetchQuery
};

In this PR, I updated the entrypoints of react-relay and relay-runtime to allow Node.js's cjs named export detection to work properly. I added a test case to the Gulp build process since to test this we need the final transpiled code, but I am open to other approaches.

@meta-cla meta-cla Bot added the CLA Signed label Feb 5, 2026
@robrichard robrichard force-pushed the robrichard/cjs-named-exports branch from 9828617 to 28d1934 Compare February 12, 2026 14:17
Comment thread scripts/testNamedExports.js Outdated
Comment thread scripts/testNamedExports.js Outdated
{file: 'relay-runtime/lib/index.js', expectedExports: 92},
{file: 'react-relay/lib/index.js', expectedExports: 35},
{file: 'react-relay/lib/hooks.js', expectedExports: 24},
{file: 'react-relay/lib/legacy.js', expectedExports: 16},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These counts feel brittle. Do we have to come update this magic number anytime we add/remove an export from one of these modules? If that's going to be the case, we likely need comments in each of the export files explaining this coupling.

Is there not some other way to enforce this? ESLint maybe?

Copy link
Copy Markdown
Contributor

@captbaritone captbaritone Apr 2, 2026

Choose a reason for hiding this comment

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

Or perhaps even better, some Babel transform or similar that can automatically produce the right output from our existing code.

Copy link
Copy Markdown
Contributor Author

@robrichard robrichard Apr 2, 2026

Choose a reason for hiding this comment

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

@captbaritone I removed this test entirely and added a an eslint rule to eslint-plugin-relay-internal. It's turned off by default and enabled only in the package entry files. Let me know what you think.

I went with a lint rule because I'm not sure I'd be able to cover every conceivable case in a babel transform.

@robrichard robrichard force-pushed the robrichard/cjs-named-exports branch 2 times, most recently from 104b754 to a6a116d Compare April 2, 2026 18:03
@robrichard robrichard force-pushed the robrichard/cjs-named-exports branch from a6a116d to f72616e Compare April 2, 2026 18:06
@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented Apr 2, 2026

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

@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented Apr 15, 2026

@captbaritone merged this pull request in 9240507.

@jantimon
Copy link
Copy Markdown
Contributor

@captbaritone instead of adding workarounds could we please move forward with #4548 instead?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants