[rush] Design note: Subspace configuration improvements

[octogonz]

trafficstars

Today @william2958 and I chatted about some remaining design questions for the Subspace config files, related to his PR https://github.com/microsoft/rushstack/pull/4715:

subspaces.md

Here's a summary of the proposed changes:

• The rush init template for .npmrc needs a # comment clearly explaining how the subspace .npmrc gets merged with common/config/rush/.npmrc

• The subspaces/<name>/.pnpmfile-subspace.cjs file (inherited from split workspaces) should get renamed to .pnpmfile.cjs, since we forbid comnmon/config/rush/.pnpmfile.cjs when the feature is enabled

• common/config/rush/pnpm-config.json should be treated as an all-or-nothing fallback in the case where subspaces/<name>/pnpm-config.json is absent.

  @octogonz will double-check with the maintainers whether this is intuitive enough or needs some clarification (such as subnspaces/_fallback/pnpm-config.json or common/config/rush/pnpm-config-fallback.json)

• ensureConsistentVersions always applies within a subspace; there is no mode where it applies across all projects in the monorepo

• The ensureConsistentVersions setting should be moved from rush.json to common-versions.json so it is alongside allowedAlternativeVersions

• --subspace is mandatory for rush check when the feature is enabled

• rush version, rush install, etc always need to invoke rush check in a loop for each subspace with ensureConsistentVersions=true (or if we are optimizing, for the subset of subspaces affected by that operation).

• rush version should NOT support the --subspace parameter` since its operation isn't associated with any particular subspace

@iclanton @chengcyber

[chengcyber]

I am excited to see this improvement! I would like to put some thoughts here:

1. Is there any pattern to share code in subspace pnpmfile.js across different subspaces? If so, is it point to the JS fashion require('relative_path/shared_pnpmfile.cjs')?

2. In the case where subspaces//pnpm-config.json is absent.

Is it say that the file-level absent? Or it means that property level absent in subspace's pnpm-config.json.

For example, useWorkspaces seems to be a monorepo-level configuration, and it cannot be set in subspace-level pnpm-config.json. Meanwhile, globalPnpmOverrides works great in subspace-level.

3. The ensureConsistentVersions setting should be moved from rush.json to common-versions.json

strongly agree!

4. rush version, rush install, etc always need to invoke rush check in a loop for every subspace (or else for a subset of subspaces affected by the operation).

If a certain subspace is ensureConsistentVersions disabled, it won't run rush check for this subpsace. Am I understanding it correctly?

[octogonz]

1. Is there any pattern to share code in subspace pnpmfile.js across different subspaces? If so, is it point to the JS fashion require('relative_path/shared_pnpmfile.cjs')?

@william2958 seemed to think this was not a useful feature, at least in TikTok's monorepo where there are hundreds of subspaces. Although you could technically share logic using require('../../common-stuff'), Rush/PNPM has no way to detect changes to files referenced in that way, which will break the rush install analysis. Also, long term, I think we want to strongly discourage usage of .pnpmfile.cjs (code) in favor of pnpm-config.json (data) as it is easier to validate and cache.

[octogonz]

Is it say that the file-level absent? Or it means that property level absent in subspace's pnpm-config.json.

"in the case where subspaces/<name>/pnpm-config.json is absent" means the entire file is absent. "all-or-nothing fallback" means all fields or no fields.

For example, useWorkspaces seems to be a monorepo-level configuration, and it cannot be set in subspace-level pnpm-config.json. Meanwhile, globalPnpmOverrides works great in subspace-level.

The useWorksapces and subspacesEnabled settings are expected to be removed in Rush 6 (always on). In the latest Rush, subspacesEnabled will produce an error if useWorkspaces=false for any subspace, so #4720 means that if you create a subspace-level pnpm-config.json then you must set useWorkspaces=true in that file also.

(BTW it would be technically possible to support useWorkspaces=false with subspaces, and even allow true/false for different subspaces, but we don't want to invest in that as our goal is to eliminate useWorkspaces=false.)

[octogonz]

4. rush version, rush install, etc always need to invoke rush check in a loop for every subspace (or else for a subset of subspaces affected by the operation).

If a certain subspace is ensureConsistentVersions disabled, it won't run rush check for this subpsace. Am I understanding it correctly?

Yes. I've clarified the wording.

[octogonz]

@william2958 Here's how the docs update will look to reflect these changes:

https://github.com/microsoft/rushstack-websites/pull/235