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.

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

[rush-lib] Add a config to pnpm-config.json about injected install for cross subspace workspace dependencies #4674

Merged
merged 8 commits into from
May 8, 2024
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,8 @@ jobs:
- name: Ensure repo README is up-to-date
run: node repo-scripts/repo-toolbox/lib/start.js readme --verify

- name: Rush update (rush-lib)
run: node apps/rush/lib/start-dev.js update --recheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

@g-chao It is technically a bug that --recheck is needed here. I bet you can solve that by adding the pnpm-sync version to ILastInstallFlagJson, but we can try that in a separate PR to avoid holding up your work.


- name: Rush test (rush-lib)
run: node apps/rush/lib/start-dev.js test --verbose --production --timeline
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Add a new setting `alwaysInjectDependenciesFromOtherSubspaces` in pnpm-config.json",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
8 changes: 4 additions & 4 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": "e1449a656b11eb9bb4b8a68763cb9727b029a19d",
"pnpmShrinkwrapHash": "84413150c512b92dfbc986fab412bca06be42ab1",
"preferredVersionsHash": "ce857ea0536b894ec8f346aaea08cfd85a5af648"
}
2 changes: 2 additions & 0 deletions common/reviews/api/rush-lib.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ export interface IPnpmLockfilePolicies {
// @internal
export interface _IPnpmOptionsJson extends IPackageManagerOptionsJsonBase {
alwaysFullInstall?: boolean;
alwaysInjectDependenciesFromOtherSubspaces?: boolean;
autoInstallPeers?: boolean;
globalAllowedDeprecatedVersions?: Record<string, string>;
globalNeverBuiltDependencies?: string[];
Expand Down Expand Up @@ -1049,6 +1050,7 @@ export class PhasedCommandHooks {
// @public
export class PnpmOptionsConfiguration extends PackageManagerOptionsConfigurationBase {
readonly alwaysFullInstall: boolean | undefined;
readonly alwaysInjectDependenciesFromOtherSubspaces: boolean | undefined;
readonly autoInstallPeers: boolean | undefined;
readonly globalAllowedDeprecatedVersions: Record<string, string> | undefined;
readonly globalNeverBuiltDependencies: string[] | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,28 @@
*/
/*[LINE "HYPOTHETICAL"]*/ "preventManualShrinkwrapChanges": true,

/**
* When a project uses `workspace:` to depend on another Rush project, PNPM normally installs
* it by creating a symlink under `node_modules`. This generally works well, but in certain
* cases such as differing `peerDependencies` versions, symlinking may cause trouble
* such as incorrectly satisfied versions. For such cases, the dependency can be declared
* as "injected", causing PNPM to copy its built output into `node_modules` like a real
* install from a registry. Details here: https://rushjs.io/pages/advanced/injected_deps/
*
* When using Rush subspaces, these sorts of versioning problems are much more likely if
* `workspace:` refers to a project from a different subspace. This is because the symlink
* would point to a separate `node_modules` tree installed by a different PNPM lockfile.
* A comprehensives solution is to enable `alwaysInjectDependenciesFromOtherSubspaces`,
* which automatically treats all projects from other subspaces as injected dependencies
* without having to manually configure them.
*
* NOTE: Use carefully -- excessive file copying can slow down the `rush install` and
* `pnpm-sync` operations if too many dependencies become injected.
*
* The default value is false.
*/
"alwaysInjectDependenciesFromOtherSubspaces": false,

/**
* Defines the policies to be checked for the `pnpm-lock.yaml` file.
*/
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"tar": "~6.2.1",
"true-case-path": "~2.2.1",
"uuid": "~8.3.2",
"pnpm-sync-lib": "0.2.2"
"pnpm-sync-lib": "0.2.4"
},
"devDependencies": {
"@pnpm/logger": "4.0.0",
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/cli/RushXCommandLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class RushXCommandLine {
);
await pnpmSyncCopyAsync({
pnpmSyncJsonPath,
ensureFolder: FileSystem.ensureFolderAsync,
ensureFolderAsync: FileSystem.ensureFolderAsync,
forEachAsyncWithConcurrency: Async.forEachAsync,
getPackageIncludedFiles: PackageExtractor.getPackageIncludedFilesAsync,
logMessageCallback: (logMessageOptions: ILogMessageCallbackOptions) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ export class WorkspaceInstallManager extends BaseInstallManager {
await pnpmSyncPrepareAsync({
lockfilePath: pnpmLockfilePath,
dotPnpmFolder,
ensureFolder: FileSystem.ensureFolderAsync,
ensureFolderAsync: FileSystem.ensureFolderAsync,
readPnpmLockfile: async (lockfilePath: string) => {
const wantedPnpmLockfile: PnpmShrinkwrapFile | undefined = await PnpmShrinkwrapFile.loadFromFile(
lockfilePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class PnpmSyncCopyOperationPlugin implements IPhasedCommandPlugin {
);
await pnpmSyncCopyAsync({
pnpmSyncJsonPath,
ensureFolder: FileSystem.ensureFolderAsync,
ensureFolderAsync: FileSystem.ensureFolderAsync,
forEachAsyncWithConcurrency: Async.forEachAsync,
getPackageIncludedFiles: PackageExtractor.getPackageIncludedFilesAsync,
logMessageCallback: (logMessageOptions: ILogMessageCallbackOptions) =>
Expand Down
17 changes: 17 additions & 0 deletions libraries/rush-lib/src/logic/pnpm/PnpmOptionsConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ export interface IPnpmOptionsJson extends IPackageManagerOptionsJsonBase {
* {@inheritDoc PnpmOptionsConfiguration.autoInstallPeers}
*/
autoInstallPeers?: boolean;
/**
* {@inheritDoc PnpmOptionsConfiguration.alwaysInjectDependenciesFromOtherSubspaces}
*/
alwaysInjectDependenciesFromOtherSubspaces?: boolean;
/**
* {@inheritDoc PnpmOptionsConfiguration.alwaysFullInstall}
*/
Expand Down Expand Up @@ -249,6 +253,18 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
*/
public readonly autoInstallPeers: boolean | undefined;

/**
* If true, then `rush update` add injected install options for all cross-subspace
* workspace dependencies, to avoid subspace doppelganger issue.
*
* Here, the injected install refers to PNPM's PNPM's "injected dependencies"
* feature. Learn more: https://pnpm.io/package_json#dependenciesmeta
*
* @remarks
* The default value is false.
*/
public readonly alwaysInjectDependenciesFromOtherSubspaces: boolean | undefined;

/**
* The "globalOverrides" setting provides a simple mechanism for overriding version selections
* for all dependencies of all projects in the monorepo workspace. The settings are copied
Expand Down Expand Up @@ -389,6 +405,7 @@ export class PnpmOptionsConfiguration extends PackageManagerOptionsConfiguration
this._globalPatchedDependencies = json.globalPatchedDependencies;
this.resolutionMode = json.resolutionMode;
this.autoInstallPeers = json.autoInstallPeers;
this.alwaysInjectDependenciesFromOtherSubspaces = json.alwaysInjectDependenciesFromOtherSubspaces;
this.alwaysFullInstall = json.alwaysFullInstall;
this.pnpmLockfilePolicies = json.pnpmLockfilePolicies;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { RushConfigurationProject } from '../../api/RushConfigurationProjec
import type { PnpmPackageManager } from '../../api/packageManager/PnpmPackageManager';
import { RushConstants } from '../RushConstants';
import type { Subspace } from '../../api/Subspace';
import type { PnpmOptionsConfiguration } from './PnpmOptionsConfiguration';

/**
* Loads PNPM's pnpmfile.js configuration, and invokes it to preprocess package.json files,
Expand Down Expand Up @@ -115,11 +116,13 @@ export class SubspacePnpmfileConfiguration {
const processTransitiveInjectedInstallQueue: Array<RushConfigurationProject> = [];

for (const subspaceProject of subspaceProjectsMap.values()) {
const injectedDependencySet: Set<string> = new Set();
const dependenciesMeta: IDependenciesMetaTable | undefined =
subspaceProject.packageJson.dependenciesMeta;
if (dependenciesMeta) {
for (const [dependencyName, { injected }] of Object.entries(dependenciesMeta)) {
if (injected) {
injectedDependencySet.add(dependencyName);
projectNameToInjectedDependenciesMap.get(subspaceProject.packageName)?.add(dependencyName);

//if this dependency is in the same subspace, leave as it is, PNPM will handle it
Expand All @@ -131,6 +134,24 @@ export class SubspacePnpmfileConfiguration {
}
}
}

// if alwaysInjectDependenciesFromOtherSubspaces policy is true in pnpm-config.json
// and the dependency is not injected yet
// and the dependency is in another subspace
// then, make this dependency as injected dependency
const pnpmOptions: PnpmOptionsConfiguration | undefined =
subspace.getPnpmOptions() || rushConfiguration.pnpmOptions;
if (pnpmOptions && pnpmOptions.alwaysInjectDependenciesFromOtherSubspaces) {
const dependencyProjects: ReadonlySet<RushConfigurationProject> = subspaceProject.dependencyProjects;
for (const dependencyProject of dependencyProjects) {
const dependencyName: string = dependencyProject.packageName;
if (!injectedDependencySet.has(dependencyName) && !subspaceProjectsMap.has(dependencyName)) {
projectNameToInjectedDependenciesMap.get(subspaceProject.packageName)?.add(dependencyName);
// process transitive injected installation
processTransitiveInjectedInstallQueue.push(workspaceProjectsMap.get(dependencyName)!);
}
}
}
}

// rewrite all workspace dependencies to injected install all for transitive injected installation case
Expand Down
5 changes: 5 additions & 0 deletions libraries/rush-lib/src/schemas/pnpm-config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
"type": "boolean"
},

"alwaysInjectDependenciesFromOtherSubspaces": {
"description": "When a project uses `workspace:` to depend on another Rush project, PNPM normally installs it by creating a symlink under `node_modules`. This generally works well, but in certain cases such as differing `peerDependencies` versions, symlinking may cause trouble such as incorrectly satisfied versions. For such cases, the dependency can be declared as \"injected\", causing PNPM to copy its built output into `node_modules` like a real install from a registry. Details here: https://rushjs.io/pages/advanced/injected_deps/\n\nWhen using Rush subspaces, these sorts of versioning problems are much more likely if `workspace:` refers to a project from a different subspace. This is because the symlink would point to a separate `node_modules` tree installed by a different PNPM lockfile. A comprehensives solution is to enable `alwaysInjectDependenciesFromOtherSubspaces`, which automatically treats all projects from other subspaces as injected dependencies without having to manually configure them.\n\nNOTE: Use carefully -- excessive file copying can slow down the `rush install` and `pnpm-sync` operations if too many dependencies become injected.\n\nThe default value is false.",
"type": "boolean"
},

"globalOverrides": {
"description": "The \"globalOverrides\" setting provides a simple mechanism for overriding version selections for all dependencies of all projects in the monorepo workspace. The settings are copied into the `pnpm.overrides` field of the `common/temp/package.json` file that is generated by Rush during installation.\n\nOrder of precedence: `.pnpmfile.cjs` has the highest precedence, followed by `unsupportedPackageJsonSettings`, `globalPeerDependencyRules`, `globalPackageExtensions`, and `globalOverrides` has lowest precedence.\n\nPNPM documentation: https://pnpm.io/package_json#pnpmoverrides",
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion rush.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* path segment in the "$schema" field for all your Rush config files. This will ensure
* correct error-underlining and tab-completion for editors such as VS Code.
*/
"rushVersion": "5.120.2",
"rushVersion": "5.122.0",

/**
* The next field selects which package manager should be installed and determines its version.
Expand Down