-
Notifications
You must be signed in to change notification settings - Fork 659
[rush-sdk]: add named export support for CommonJS compatibility #5539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit enhances @rushstack/rush-sdk to support named imports when the package is consumed via ESM project.
common/changes/@microsoft/rush/chore-optimize-named-exports_2026-01-06-12-36.json
Outdated
Show resolved
Hide resolved
| { | ||
| "name": "cjs-module-lexer", | ||
| "allowedCategories": [ "libraries" ] | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go in nonbrowser-approved-packages.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emmm. It's generated automatically. Do you mean that I need to manually move the config into nonbrowser-approved-packages.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly.
| import * as path from 'node:path'; | ||
|
|
||
| import { FileSystem, Import, Path } from '@rushstack/node-core-library'; | ||
| import { initSync, parse } from 'cjs-module-lexer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're already generating types with named exports and ESM modules, so we shouldn't need to try to infer the names from the CJS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does your suggestion involve reading .d.ts files to confirm what named exports are available? Which of the following approaches do you think is better:
- ast-grep
- TypeScript
- RegExp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at https://www.npmjs.com/package/es-module-lexer and analyzing the ESM modules we already generate in rush-lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for using [email protected] here is that Node.js also uses this version, ensuring that the export identifiers we add can remain consistent with the actual Node.js runtime (equivalent to letting Node.js run first to see exactly which export names it can recognize). If different lexer implementations or lexer-executed source code are used, there might be some risk of inconsistency with the actual runtime, potentially leading to runtime issues. However, the risk seems acceptable.
…26-01-06-12-36.json Co-authored-by: Ian Clanton-Thuon <[email protected]>
Summary
This commit enhances @rushstack/rush-sdk to support named imports when the package is consumed via ESM project.
Resolve #5506
Details
Here's the changes.
libraries/rush-sdk/package.json
[email protected]as devDependency. Required for extracting named exports during stub generation. Version2.1.0is consistent with the built-in version in Node.js.libraries/rush-sdk/config/jest.config.json
libraries/rush-sdk/src/generate-stubs.ts
extractNamedExports(source)function to parse CommonJS modules and extract export names.libraries/rush-sdk/src/test/snapshots/script.test.ts.snap
libraries/rush-sdk/src/test/script.test.ts
libraries/rush-sdk/src/test/build-assets-with-named-exports.test.ts (NEW)
lib-shim/andlib/assets.libraries/rush-sdk/webpack.config.js
webpack.BannerPluginimport from webpack. Reads all export specifiers from@microsoft/rush-liband inject the dynamic exports placeholder code by using thsBannerPlugin.The core improvement enables consumers to use both default imports and named destructuring:
How it was tested
Unit tests were added.
Impacted documentation
None