Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Chore: add named exports to support for `@rushstack/rush-sdk` used via ESM named import.",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
4 changes: 4 additions & 0 deletions common/config/rush/browser-approved-packages.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
"name": "axios",
"allowedCategories": [ "libraries" ]
},
{
"name": "cjs-module-lexer",
"allowedCategories": [ "libraries" ]
},
Comment on lines +61 to +64
Copy link
Member

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

Copy link
Contributor Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Exactly.

{
"name": "dependency-path",
"allowedCategories": [ "libraries" ]
Expand Down
12 changes: 6 additions & 6 deletions common/config/subspaces/build-tests-subspace/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush.
{
"pnpmShrinkwrapHash": "32f13ef1f15898a4f614bf9897cc1d74d8fdf2dd",
"pnpmShrinkwrapHash": "402417ff6a3ef549c064c379cb9793ec4b4d64af",
"preferredVersionsHash": "550b4cee0bef4e97db6c6aad726df5149d20e7d9",
"packageJsonInjectedDependenciesHash": "cb59d652ae8cf04249e1fa557d15d2958128a5e8"
"packageJsonInjectedDependenciesHash": "248fe4df023dec4d802dbb3f8d3f90842fd458ed"
}
42 changes: 25 additions & 17 deletions common/config/subspaces/default/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion common/config/subspaces/default/repo-state.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush.
{
"pnpmShrinkwrapHash": "02e03149d5bf0b6a4e8dd2df668ed5350b0506c5",
"pnpmShrinkwrapHash": "f91ada7a0ec37139e2f0cff3fb869d1f514ee628",
"preferredVersionsHash": "a9b67c38568259823f9cfb8270b31bf6d8470b27"
}
4 changes: 2 additions & 2 deletions libraries/rush-sdk/config/jest.config.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"extends": "local-node-rig/profiles/default/config/jest.config.json",

"roots": ["<rootDir>/lib-shim"],
"roots": ["<rootDir>/lib-commonjs"],

"testMatch": ["<rootDir>/lib-shim/**/*.test.js"],
"testMatch": ["<rootDir>/lib-commonjs/**/*.test.js"],

"collectCoverageFrom": [
"lib-shim/**/*.js",
Expand Down
1 change: 1 addition & 0 deletions libraries/rush-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@rushstack/webpack-preserve-dynamic-require-plugin": "workspace:*",
"@types/semver": "7.5.0",
"@types/webpack-env": "1.18.8",
"cjs-module-lexer": "2.1.0",
"eslint": "~9.37.0",
"local-node-rig": "workspace:*",
"webpack": "~5.103.0"
Expand Down
22 changes: 20 additions & 2 deletions libraries/rush-sdk/src/generate-stubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

import * as path from 'node:path';

import { FileSystem, Import, Path } from '@rushstack/node-core-library';
import { initSync, parse } from 'cjs-module-lexer';
Copy link
Member

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.

Copy link
Contributor Author

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:

  1. ast-grep
  2. TypeScript
  3. RegExp

Copy link
Member

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.

Copy link
Contributor Author

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.


import { Encoding, FileSystem, Import, Path } from '@rushstack/node-core-library';

function generateLibFilesRecursively(options: {
parentSourcePath: string;
Expand All @@ -14,6 +16,10 @@ function generateLibFilesRecursively(options: {
for (const folderItem of FileSystem.readFolderItems(options.parentSourcePath)) {
const sourcePath: string = path.join(options.parentSourcePath, folderItem.name);
const targetPath: string = path.join(options.parentTargetPath, folderItem.name);
const commonjsPath: string = path.join(
options.parentSourcePath.replace('/rush-lib/lib', '/rush-lib/lib-commonjs'),
folderItem.name
);

if (folderItem.isDirectory()) {
// create destination folder
Expand All @@ -36,11 +42,17 @@ function generateLibFilesRecursively(options: {
const shimPathLiteral: string = JSON.stringify(Path.convertToSlashes(shimPath));
const srcImportPathLiteral: string = JSON.stringify(srcImportPath);

const sourceCode: string = FileSystem.readFile(commonjsPath, { encoding: Encoding.Utf8 });
const exportedNames: string[] = extractNamedExports(sourceCode);
const namedExportsPlaceholder: string = exportedNames.length
? `${exportedNames.map((name) => `exports.${name}`).join(' = ')} = undefined;\n\n`
: '';

FileSystem.writeFile(
targetPath,
// Example:
// module.exports = require("../../../lib-shim/index")._rushSdk_loadInternalModule("logic/policy/GitEmailPolicy");
`module.exports = require(${shimPathLiteral})._rushSdk_loadInternalModule(${srcImportPathLiteral});`
`${namedExportsPlaceholder}module.exports = require(${shimPathLiteral})._rushSdk_loadInternalModule(${srcImportPathLiteral});`
);
}
}
Expand All @@ -58,6 +70,7 @@ export async function runAsync(): Promise<void> {
const stubsTargetPath: string = path.resolve(__dirname, '../lib');
// eslint-disable-next-line no-console
console.log('generate-stubs: Generating stub files under: ' + stubsTargetPath);
initSync();
generateLibFilesRecursively({
parentSourcePath: path.join(rushLibFolder, 'lib'),
parentTargetPath: stubsTargetPath,
Expand All @@ -67,3 +80,8 @@ export async function runAsync(): Promise<void> {
// eslint-disable-next-line no-console
console.log('generate-stubs: Completed successfully.');
}

export function extractNamedExports(source: string): string[] {
const { exports, reexports } = parse(source);
return [...exports, ...reexports].filter((d) => d !== '__esModule');
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Loaded @microsoft/rush-lib from process.env._RUSH_LIB_PATH
'VersionPolicyDefinitionName',
'YarnOptionsConfiguration',
'_FlagFile',
'_OperationBuildCache',
'_OperationMetadataManager',
'_OperationStateFile',
'_RushGlobalFolder',
Expand All @@ -76,15 +77,6 @@ exports[`@rushstack/rush-sdk Should load via global (for plugins): stdout 1`] =

exports[`@rushstack/rush-sdk Should load via install-run (for standalone tools): stderr 1`] = `""`;

exports[`@rushstack/rush-sdk Should load via install-run (for standalone tools): stdout 1`] = `
"Trying to load @microsoft/rush-lib installed by install-run-rush
Loaded @microsoft/rush-lib installed by install-run-rush
[
'_rushSdk_loadInternalModule',
'foo'
]"
`;

exports[`@rushstack/rush-sdk Should load via process.env._RUSH_LIB_PATH (for child processes): stderr 1`] = `""`;

exports[`@rushstack/rush-sdk Should load via process.env._RUSH_LIB_PATH (for child processes): stdout 1`] = `
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { Executable } from '@rushstack/node-core-library';

describe('@rushstack/rush-sdk named exports check', () => {
it('Should import named exports correctly (lib-shim)', () => {
const result = Executable.spawnSync('node', [
'-e',
`
const { RushConfiguration } = await import('@rushstack/rush-sdk');
console.log(typeof RushConfiguration.loadFromConfigurationFile);
`
]);
expect(result.stdout.trim()).toEqual('function');
expect(result.status).toBe(0);
});

it('Should import named exports correctly (lib)', () => {
const result = Executable.spawnSync('node', [
'-e',
`
const { RushConfiguration } = await import('@rushstack/rush-sdk/lib/api/RushConfiguration');
console.log(typeof RushConfiguration.loadFromConfigurationFile);
`
]);
expect(result.stdout.trim()).toEqual('function');
expect(result.status).toBe(0);
});
});
4 changes: 3 additions & 1 deletion libraries/rush-sdk/src/test/script.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ ${loadAndPrintRushSdkModule}
}
);
expect(result.stderr.trim()).toMatchSnapshot('stderr');
expect(result.stdout.trim()).toMatchSnapshot('stdout');
expect(result.stdout.trim()).toContain(
'Trying to load @microsoft/rush-lib installed by install-run-rush'
);
expect(result.status).toBe(0);
});
});
Loading
Loading