fix(ext/node): re-export inner spec for module.exports = require(X).Y by bartlomieju · Pull Request #34363 · denoland/deno

bartlomieju · 2026-05-25T16:32:41Z

deno_ast's CJS analyzer recognizes the bare-call form
module.exports = require("./inner") and surfaces the inner specifier as
a re-export, but the very common member-access form
module.exports = require("./inner").Y falls through unhandled. As a
result, packages whose main entry has that shape (graphql-tag@2 is the
canonical example) expose no named exports — import { gql } from "npm:graphql-tag" fails with "does not provide an export named 'gql'".

When static analysis comes back with no exports and no re-exports, walk
the program's top-level statements looking for the
module.exports = require(LIT)[.IDENT]* pattern and treat the require
specifier as a re-export. The existing recursive re-export machinery
then resolves the inner module and picks up its named exports, which
produces the same set of names Node exposes for these packages.

Fixes #25311

deno_ast's CJS analyzer recognizes `module.exports = require("./inner")` but not the member-access form `module.exports = require("./inner").Y`, which causes packages like graphql-tag@2 to surface no named exports — `import { gql } from "npm:graphql-tag"` fails with "does not provide an export named 'gql'". When static analysis returns no exports and no reexports, scan the program's top-level statements for the `module.exports = require(LIT)[.IDENT]*` pattern and surface the require specifier as a re-export. The existing recursive re-export machinery then picks up the inner module's named exports. Fixes #25311.

fibibot

reexports.push(spec) treats every module.exports = require("x").member assignment as if the module re-exported all of x. That over-exposes named exports when member is only a sub-object, for example module.exports = require("node:crypto").webcrypto would advertise names from node:crypto instead of only properties on webcrypto. Please narrow this to the specific member shape you can prove is safe, or add a guard/test that prevents importing unrelated names through the wrapper.

Addresses the review feedback on the previous commit: treating every `module.exports = require(X).MEMBER` shape as a wildcard re-export of all of `X`'s named exports over-exposed names that may not actually be properties of `MEMBER`. For example, a wrapper that does `module.exports = require("node:crypto").webcrypto` would advertise names like `createHash` that aren't reachable through `webcrypto`. Carry the member name through analysis (new `member_reexports` on `ModuleExportsAndReExports` / `CjsAnalysisExports`). At the recursive analyze step, resolve the inner specifier, look up the identifier bound to `exports.MEMBER` in the inner module, and advertise only the property names statically assigned to that identifier. When the value can't be statically resolved to such an identifier, advertise nothing under the member shape. A regression test fixture demonstrates both: properties attached to the member surface as named exports, while a sibling named export that the member does not carry is correctly not advertised.

bartlomieju · 2026-05-26T09:04:06Z

@fibibot Addressed in b036048. The previous commit treated every module.exports = require(X).MEMBER shape as a wildcard re-export of all of X's named exports, which over-exposed names that aren't actually properties of MEMBER (the node:crypto.webcrypto case you flagged).

Now the analyzer carries the member name through (member_reexports on ModuleExportsAndReExports / CjsAnalysisExports) and the recursive analyze step resolves the inner specifier, finds the identifier bound to exports.MEMBER in the inner module, and only advertises the property names statically assigned to that identifier (IDENT.X = ...). When the member value can't be resolved to such an identifier, no names are advertised under the member shape.

Added a regression fixture (@denotest/cjs-module-exports-require-member-narrow) where inner exposes a named export (loose) that is NOT a property of the re-exported member (safe). The loose_is_not_advertised spec test asserts that import { loose } from ... fails at runtime with "does not provide an export named 'loose'", and the attached_works test confirms that names actually attached to the member still surface.

fibibot

One blocker remains: member-shaped reexports are resolved for the entry module, but not for modules reached through normal CJS reexports.

analysis.member_reexports is handled for the top-level analysis before returning from analyze_all_exports, but the recursive branch in analyze_reexports only follows analysis.reexports and extends analysis.exports. For index.js: module.exports = require("./wrapper") where wrapper.js is module.exports = require("./inner").safe, the wrapper function

fibibot

One blocker remains: member-shaped reexports are resolved for the entry module, but not for modules reached through normal CJS reexports.

analysis.member_reexports is handled for the top-level analysis before returning from analyze_all_exports, but the recursive branch in analyze_reexports only follows analysis.reexports and extends analysis.exports. For index.js: module.exports = require("./wrapper") where wrapper.js is module.exports = require("./inner").safe, the wrapper function's attached property is never advertised. Call resolve_member_reexports(&reexport_specifier, &analysis.member_reexports, ...) in that branch too, and add a nested fixture for this shape.

The entry-module path called `resolve_member_reexports` for member-shaped re-exports, but the recursive branch in `analyze_reexports` only walked `analysis.reexports` and extended `analysis.exports`, ignoring `analysis.member_reexports` on modules reached through normal CJS re-exports. So for `main.js: module.exports = require("./wrapper")` where `wrapper.js` is `module.exports = require("./inner").safe`, the wrapper's narrowed names never surfaced on the entry. Resolve member re-exports inside the recursive loop too, using the wrapper as the referrer. Add a nested fixture (`@denotest/cjs-module-exports-require-member-nested`) and spec tests covering both that the wrapper's attached property is advertised through the chain and that the inner module's unrelated `loose` export is not.

fibibot

The new head fixes the recursive blocker: analyze_reexports() now calls resolve_member_reexports(&reexport_specifier, &analysis.member_reexports, ...) before extending exports, so module.exports = require("./wrapper") picks up names attached to require("./inner").safe. The nested fixture covers both attached being advertised and loose staying hidden. Holding approval until CI is green; the run is still in progress and deno_core test linux-x86_64 is red without logs available yet.

fibibot

CI is red on deno_core test linux-x86_64; that shard is outside this PR's CJS export-analysis path, and gh run view --log-failed did not expose a concrete failure log. CI red; flake-watcher will triage, so I'm still not approving this head.

fibibot

CI is green now, promoting prior review to APPROVE.

dsherret · 2026-05-28T13:50:58Z

+  }
+}
+
+fn find_module_exports_require_member(


Why is all this code here instead of in deno_ast?

Edit: Ah, actually I guess it is different and this is probably fine. It just feels like this should be in deno_ast with the rest of the cjs analysis though.

dsherret · 2026-05-28T13:53:06Z

+      let parsed = module_export_analyzer
+        .parse_module(specifier, media_type, source_arc)?;


Seems inefficient. Maybe I'm not getting why parsing the whole module is required. Won't this potentially cause a module to be parsed many times?

dsherret · 2026-05-28T13:57:48Z

+    let ident = find_member_value_ident(&stmts, member)?;
+
+    // Collect property names statically assigned to that identifier
+    // at the top level: `IDENT.X = ...`.
+    let mut props: Vec<String> = stmts
+      .iter()
+      .filter_map(|stmt| match_identifier_property_assignment(stmt, &ident))


This loops over the statements twice. I feel like it could do it once?

Follow-up to #34363

…#34689) Importing a named export from graphql-tag still failed with "does not provide an export named 'gql'", even though #34363 added handling for the `module.exports = require("X").MEMBER` entry shape that graphql-tag@2 uses. That fix narrows the wrapper's named exports to the names statically attached to MEMBER inside X, which works when the attachments are top-level but not for graphql-tag's actual inner file: a UMD bundle that performs its `exports.gql = ...` and `gql.* = ...` assignments inside the factory IIFE and builds the value through a namespace alias. Static narrowing finds nothing there, so the wrapper advertised no names at all. When the member's attached names cannot be determined statically, fall back to re-exporting the inner module wholesale rather than advertising nothing. This matches Node, which over-approximates `require(X).Y` to the inner module's full set of named exports. The narrow path is unchanged for the cases where the attachments are statically known, so unrelated names still do not leak through there. Closes #16708 Closes #25311

…denoland#34363) deno_ast's CJS analyzer recognizes the bare-call form `module.exports = require("./inner")` and surfaces the inner specifier as a re-export, but the very common member-access form `module.exports = require("./inner").Y` falls through unhandled. As a result, packages whose main entry has that shape (graphql-tag@2 is the canonical example) expose no named exports — `import { gql } from "npm:graphql-tag"` fails with "does not provide an export named 'gql'". When static analysis comes back with no exports and no re-exports, walk the program's top-level statements looking for the `module.exports = require(LIT)[.IDENT]*` pattern and treat the require specifier as a re-export. The existing recursive re-export machinery then resolves the inner module and picks up its named exports, which produces the same set of names Node exposes for these packages. Fixes denoland#25311

Follow-up to denoland#34363

bartlomieju changed the title ~~fix(node): re-export inner spec for module.exports = require(X).Y~~ fix(ext/node): re-export inner spec for module.exports = require(X).Y May 25, 2026

fibibot requested changes May 25, 2026

View reviewed changes

fibibot requested changes May 26, 2026

View reviewed changes

fibibot reviewed May 27, 2026

View reviewed changes

fibibot approved these changes May 27, 2026

View reviewed changes

bartlomieju merged commit fd5c160 into main May 28, 2026
268 of 270 checks passed

bartlomieju deleted the fix/cjs-runtime-export-introspection branch May 28, 2026 13:18

dsherret reviewed May 28, 2026

View reviewed changes

bartlomieju mentioned this pull request May 28, 2026

perf(ext/node): cache member-export-props analysis #34471

Merged

bartlomieju added a commit that referenced this pull request May 30, 2026

perf(ext/node): cache member-export-props analysis (#34471)

8fd78d5

Follow-up to #34363

bartlomieju mentioned this pull request Jun 1, 2026

fix(node): full re-export fallback for unresolvable member re-exports #34689

Merged

bartlomieju mentioned this pull request Jun 2, 2026

fix(publish): improve error reporting for JSR manifest verification #34690

Merged

littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026

perf(ext/node): cache member-export-props analysis (denoland#34471)

2ee2865

Follow-up to denoland#34363

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ext/node): re-export inner spec for module.exports = require(X).Y#34363

fix(ext/node): re-export inner spec for module.exports = require(X).Y#34363
bartlomieju merged 3 commits into
mainfrom
fix/cjs-runtime-export-introspection

bartlomieju commented May 25, 2026

Uh oh!

fibibot left a comment

Uh oh!

bartlomieju commented May 26, 2026

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

Uh oh!

dsherret May 28, 2026 •

edited

Loading

Uh oh!

dsherret May 28, 2026 •

edited

Loading

Uh oh!

dsherret May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let parsed = module_export_analyzer
		.parse_module(specifier, media_type, source_arc)?;

Uh oh!

Conversation

bartlomieju commented May 25, 2026

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

bartlomieju commented May 26, 2026

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dsherret May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsherret May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsherret May 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dsherret May 28, 2026 •

edited

Loading

dsherret May 28, 2026 •

edited

Loading