[rush] PNPM Overrides with more advanced syntax will cause Rush to always complain that shrinkwrap file need to be updated about rushstack HOT 4 OPEN

We discussed this and I think arrived at two separate decisions:

1. Create a library that performs accurate transforms. PnpmfileConfiguration.ts should be split into two separate APIs: One half is the Rush-specific API whose purpose is only to generate the .pnpmfile.cjs shim. The other half will be a common library whose purpose is to accurately transform a package.json file the way that PNPM would do. This involves loading .pnpmfile.cjs (ideally into an isolated web worker that can be cleaned up easily), as well as applying other transforms such as "overrides" that do not involve any script code, probably leveraging the @pnpm/hooks.read-package-hook package that @chengcyber mentioned.

2. Don't rely on transforms at all for rush install. Today rush install is transforming package.json because it is trying to compare project dependency versions against the lockfile to determine whether the lockfile is up-to-date. This was a good idea in the old days, but PNPM is now so complicated and so reliable that such logic has become difficult to support. A better model would be to focus purely on inputs: (1) collect all the inputs such as package.json, pnpm-config.json, .pnpmfile.cjs, etc. (2) normalize the file contents by removing irrelevant fields then sorting, (3) hash the result, and then (4) compare those hashes against hashes saved from the previous successful rush update. This avoids the need for semantic analysis of lockfiles, and so is a much simpler algorithm, and will be much more robust for behavioral changes across PNPM releases.

Thus, the proposed common library for accurate package.json transforms is really only needed by rush deploy and the Lockfile Explorer app. In both cases, although we will aim for 100% accurate transforms of package.json, it is not a big deal if there are some minor mistakes. (Lockfile Explorer is just a viewer, and rush deploy's analysis is technically a heuristic that can be fixed up using manual configurations.)

These two actions are separate work items that can be implemented in any order. #2 is probably the quickest fix for #4675.

from rushstack.

There's a TODO in the code just above where you linked to consider using globalOverrides to modify the evaluation of workspace-local package.json files to be an error:

The basis for this is that globalOverrides is intended to be a mechanism for compensating for incorrect content in package.json in external dependencies, so if the contents of a local package.json are wrong, that package.json should be directly modified.

Enforcing the installed version dependency in workspace packages is the domain of common/config/common-versions.json and the ensureConsistentVersions setting: https://rushjs.io/pages/maintainer/recommended_settings/#ensureconsistentversions

from rushstack.

I see... so this should throw an error I guess. At the moment, there's no "hint" of what went wrong and users will keep retrying to rush update without avail

from rushstack.

Hi @kenrick95

but it seems like it is not handled when it is first implemented in the PR

You are right. The initial implementation doesn't cover the advanced syntax, as it was a quick fix for the urgent issues in our company's monorepo.

I just checked this requirement again to see whether I can quickly support it. However, It seems to me a little bit complicated to support the advance syntax. Technically, PnpmfileConfiguration#transform needs to know the overrides configuration and mimic the pnpm's fashion to patch hooks.readPackage function if overrides exist

Further, to make it 100% accurate, there are other configurations should be supported as well. Related pnpm code is
https://github.com/pnpm/pnpm/blob/aa33269f9f9fc0c3505ae1c59264d1706923a971/hooks/read-package-hook/src/createReadPackageHook.ts

from rushstack.