10000 Merge pull request #4692 from microsoft/octogonz/misc-fixes · nirshar/rushstack@1cb0e96 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1cb0e96

Browse files
octogonzoctogonz
authored
Merge pull request microsoft#4692 from microsoft/octogonz/misc-fixes
[rush] Misc fixes for Rush subspaces
2 parents b7704b4 + a1f4d46 commit 1cb0e96

File tree

8 files changed

+52
-15
lines changed

8 files changed

+52
-15
lines changed

‎common/changes/@microsoft/rush/octogonz-misc-fixes_2024-05-10-03-08.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Fix some minor issues with the \"rush init\" template files",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

‎common/changes/@microsoft/rush/octogonz-misc-fixes_2024-05-10-03-09.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Report an error if subspacesFeatureEnabled=true without useWorkspaces=true",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

‎common/reviews/api/rush-lib.api.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,12 @@ export class FileSystemBuildCacheProvider {
311311
}
312312

313313
// @internal
314-
export class _FlagFile<T extends object = JsonObject> {
315-
constructor(folderPath: string, flagName: string, initialState?: T);
314+
export class _FlagFile {
315+
constructor(folderPath: string, flagName: string, initialState?: JsonObject);
316316
clearAsync(): Promise<void>;
317317
createAsync(): Promise<void>;
318318
isValidAsync(): Promise<boolean>;
319319
readonly path: string;
320-
protected _state: T | {};
321320
}
322321

323322
// @beta

‎libraries/rush-lib/assets/rush-init/common/config/rush/pnpm-config.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@
132132
* When using Rush subspaces, these sorts of versioning problems are much more likely if
133133
* `workspace:` refers to a project from a different subspace. This is because the symlink
134134
* would point to a separate `node_modules` tree installed by a different PNPM lockfile.
135-
* A comprehensives solution is to enable `alwaysInjectDependenciesFromOtherSubspaces`,
135+
* A comprehensive solution is to enable `alwaysInjectDependenciesFromOtherSubspaces`,
136136
* which automatically treats all projects from other subspaces as injected dependencies
137137
* without having to manually configure them.
138138
*
@@ -141,12 +141,12 @@
141141
*
142142
* The default value is false.
143143
*/
144-
"alwaysInjectDependenciesFromOtherSubspaces": false,
144+
/*[LINE "HYPOTHETICAL"]*/ "alwaysInjectDependenciesFromOtherSubspaces": false,
145145

146146
/**
147147
* Defines the policies to be checked for the `pnpm-lock.yaml` file.
148148
*/
149-
"pnpmLockfilePolicies": {
149+
"pnpmLockfilePolicies": {
150150

151151
/**
152152
* This policy will cause "rush update" to report an error if `pnpm-lock.yaml` contains

‎libraries/rush-lib/assets/rush-init/rush.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,13 @@
327327
*/
328328
"projectFolder": "apps/my-app",
329329

330+
/**
331+
* This field is only used if "subspacesEnabled" is true in subspaces.json.
332+
* It specifies the subspace that this project belongs to. If omitted, then the
333+
* project belongs to the "default" subspace.
334+
*/
335+
/*[LINE "HYPOTHETICAL"]*/ "subspaceName": "my-subspace",
336+
330337
/**
331338
* An optional category for usage in the "browser-approved-packages.json"
332339
* and "nonbrowser-approved-packages.json" files. The value must be one of the

‎libraries/rush-lib/src/api/FlagFile.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { objectsAreDeepEqual } from '../utilities/objectUtilities';
88
* A base class for flag file.
99
* @internal
1010
*/
11-
export class FlagFile<T extends object = JsonObject> {
11+
export class FlagFile {
1212
/**
1313
* Flag file path
1414
*/
@@ -17,14 +17,14 @@ export class FlagFile<T extends object = JsonObject> {
1717
/**
1818
* Content of the flag
1919
*/
20-
protected _state: T | {};
20+
private _state: JsonObject;
2121

2222
/**
2323
* Creates a new flag file
2424
* @param folderPath - the folder that this flag is managing
2525
* @param state - optional, the state that should be managed or compared
2626
*/
27-
public constructor(folderPath: string, flagName: string, initialState?: T) {
27+
public constructor(folderPath: string, flagName: string, initialState?: JsonObject) {
2828
this.path = `${folderPath}/${flagName}.flag`;
2929
this._state = initialState || {};
3030
}
@@ -33,10 +33,10 @@ export class FlagFile<T extends object = JsonObject> {
3333
* Returns true if the file exists and the contents match the current state.
3434
*/
3535
public async isValidAsync(): Promise<boolean> {
36-
let oldState: T | undefined;
36+
let oldState: JsonObject | undefined;
3737
try {
3838
oldState = await JsonFile.loadAsync(this.path);
39-
const newState: T | {} = this._state;
39+
const newState: JsonObject = this._state;
4040
return objectsAreDeepEqual(oldState, newState);
4141
} catch (err) {
4242
return false;

‎libraries/rush-lib/src/api/Subspace.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,23 @@ export class Subspace {
6767
*/
6868
public getPnpmOptions(): PnpmOptionsConfiguration | undefined {
6969
if (!this._cachedPnpmOptionsInitialized) {
70+
// Calculate these outside the try/catch block since their error messages shouldn't be annotated:
71+
const subspaceConfigFolder: string = this.getSubspaceConfigFolder();
72+
const subspaceTempFolder: string = this.getSubspaceTempFolder();
7073
try {
7174
this._cachedPnpmOptions = PnpmOptionsConfiguration.loadFromJsonFileOrThrow(
72-
`${this.getSubspaceConfigFolder()}/${RushConstants.pnpmConfigFilename}`,
73-
this.getSubspaceTempFolder()
75+
`${subspaceConfigFolder}/${RushConstants.pnpmConfigFilename}`,
76+
subspaceTempFolder
7477
);
7578
this._cachedPnpmOptionsInitialized = true;
7679
} catch (e) {
7780
if (FileSystem.isNotExistError(e as Error)) {
7881
this._cachedPnpmOptions = undefined;
7982
this._cachedPnpmOptionsInitialized = true;
8083
} else {
81-
throw new Error(`The subspace has an invalid pnpm-config.json file: ${this.subspaceName}`);
84+
throw new Error(
85+
`The subspace "${this.subspaceName}" has an invalid pnpm-config.json file:\n` + e.message
86+
);
8287
}
8388
}
8489
}
@@ -91,6 +96,12 @@ export class Subspace {
9196
let subspaceConfigFolder: string;
9297

9398
if (rushConfiguration.subspacesFeatureEnabled) {
99+
if (!rushConfiguration.pnpmOptions.useWorkspaces) {
100+
throw new Error(
101+
`The Rush subspaces feature is enabled. You must set useWorkspaces=true in pnpm-config.json.`
102+
);
103+
}
104+
94105
// If this subspace doesn't have a configuration folder, check if it is in the project folder itself
95106
// if the splitWorkspaceCompatibility option is enabled in the subspace configuration
96107

‎libraries/rush-lib/src/schemas/pnpm-config.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
},
5050

5151
"alwaysInjectDependenciesFromOtherSubspaces": {
52-
"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.",
52+
"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 comprehensive 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.",
5353
"type": "boolean"
5454
},
5555

0 commit comments

Comments
 (0)
0