Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[rush] preferredVersions doesn't work when subspace is enabled. #5118

Open
fzxen opened this issue Feb 20, 2025 · 1 comment · May be fixed by #5133
Open

[rush] preferredVersions doesn't work when subspace is enabled. #5118

fzxen opened this issue Feb 20, 2025 · 1 comment · May be fixed by #5133

Comments

@fzxen
Copy link
Contributor

fzxen commented Feb 20, 2025

Summary

I am trying to enable subspace in my repo by flowing the doc. Then discovered a problem: After enabling the subspace feature, the preferredVersions in common-version will not take effect.

Details

when subspace is disable, rush will create a file named as pnpmfileSettings.json including preferredVersions.

rushstack/libraries/rush-lib/src/logic/pnpm/IPnpmfile.ts

Lines 13 to 22 in e64a1e5

export interface IPnpmfileShimSettings {
semverPath: string;
allPreferredVersions: { [dependencyName: string]: string };
allowedAlternativeVersions: { [dependencyName: string]: ReadonlyArray<string> };
/**
* The versions of all packages that are part of the workspace.
*/
workspaceVersions: Record<string, string>;
userPnpmfilePath?: string;
}

Then, pnpmfile.cjs read preferredVersions from pnpmfileSettings.json and it can set the preferred versions on the dependency map

rushstack/libraries/rush-lib/src/logic/pnpm/PnpmfileShim.ts

Lines 82 to 103 in e64a1e5

function setPreferredVersions(dependencies: { [dependencyName: string]: string } | undefined): void {
for (const [name, version] of Object.entries(dependencies || {})) {
const preferredVersion: string | undefined = allPreferredVersions?.get(name);
if (preferredVersion && !allowedAlternativeVersions?.get(name)?.has(version)) {
let preferredVersionRange: TSemver.Range | undefined;
let versionRange: TSemver.Range | undefined;
try {
preferredVersionRange = new semver!.Range(preferredVersion);
versionRange = new semver!.Range(version);
} catch {
// Swallow invalid range errors
}
if (
preferredVersionRange &&
versionRange &&
semver!.subset(preferredVersionRange, versionRange, { includePrerelease: true })
) {
dependencies![name] = preferredVersion;
}
}
}
}

However, when subspace is enable, pnpmfileSettings.json no longer contains preferredVersion information

rushstack/libraries/rush-lib/src/logic/pnpm/IPnpmfile.ts

Lines 34 to 39 in e64a1e5

export interface ISubspacePnpmfileShimSettings {
semverPath: string;
workspaceProjects: Record<string, IWorkspaceProjectInfo>;
subspaceProjects: Record<string, IWorkspaceProjectInfo>;
userPnpmfilePath?: string;
}

rush create a file named as global-pnpmfile.cjs and it doesn't seem to implement any logic related to preferredVersions.

rushstack/libraries/rush-lib/src/logic/pnpm/SubspaceGlobalPnpmfileShim.ts

Lines 34 to 64 in e64a1e5

function init(context: IPnpmfileContext | any): IPnpmfileContext {
// Sometimes PNPM may provide us a context arg that doesn't fit spec, ex.:
// https://github.com/pnpm/pnpm/blob/97c64bae4d14a8c8f05803f1d94075ee29c2df2f/packages/get-context/src/index.ts#L134
// So we need to normalize the context format before we move on
if (typeof context !== 'object' || Array.isArray(context)) {
context = {
log: (message: string) => {},
originalContext: context
} as IPnpmfileContext;
}
if (!settings) {
// Initialize the settings from file
if (!context.splitWorkspacePnpmfileShimSettings) {
context.splitWorkspacePnpmfileShimSettings = __non_webpack_require__('./pnpmfileSettings.json');
}
settings = context.splitWorkspacePnpmfileShimSettings!;
} else if (!context.splitWorkspacePnpmfileShimSettings) {
// Reuse the already initialized settings
context.splitWorkspacePnpmfileShimSettings = settings;
}
// If a userPnpmfilePath is provided, we expect it to exist
if (!userPnpmfile && settings.userPnpmfilePath) {
userPnpmfile = require(settings.userPnpmfilePath);
}
// If a semverPath is provided, we expect it to exist
if (!semver && settings.semverPath) {
semver = require(settings.semverPath);
}
// Return the normalized context
return context as IPnpmfileContext;
}

Subspace does not support preferredVersions, does it?

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version? 5.147.2
rushVersion from rush.json? 5.147.2
useWorkspaces from rush.json? Yes
Operating system? Mac
Would you consider contributing a PR? Yes
Node.js version (node -v)? v20.16.0
@github-project-automation github-project-automation bot moved this to Needs triage in Bug Triage Feb 20, 2025
@fzxen
Copy link
Contributor Author

fzxen commented Feb 24, 2025

After debugging rush-lib, I found that pnpmfileSettings.json will be written twice.

First time:

rushstack/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts

Lines 80 to 83 in 438525c

// Write the settings file used by the shim
await JsonFile.saveAsync(pnpmfileShimSettings, path.join(targetDir, 'pnpmfileSettings.json'), {
ensureFolderExists: true
});

Second time:

rushstack/libraries/rush-lib/src/logic/pnpm/SubspacePnpmfileConfiguration.ts

Lines 50 to 56 in 438525c

await JsonFile.saveAsync(
subspaceGlobalPnpmfileShimSettings,
path.join(targetDir, 'pnpmfileSettings.json'),
{
ensureFolderExists: true
}
);

The second write operation will overwrite the first write content. And pnpmfileSettings.json is read once by global-pnpmfile.cjs and pnpmfile.cjs respectively.

Maybe we should distinguish the file names written twice, one is named pnpmfileSettings.json and the other named is globalPnpmfileSettings.json.

@fzxen fzxen linked a pull request Feb 25, 2025 that will close this issue
Open
@iclanton iclanton moved this from Needs triage to Needs Investigation in Bug Triage Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
Bug Triage
Status: Needs Investigation
Milestone
No milestone
1 participant
@fzxen