fix: --lockfile-only and readPackage hook modifying workspace packages

[Silic0nS0ldier]

Resolves #5670

Recursive installation will now defer to originalManifest when checking if workspace package.json need to be updated, if available.

Note that this fix applies to the scenario where pnpm-lock.yaml exists, odds are that when generating from scratch, one of the following codepaths are used instead.

• pnpm/pkg-manager/resolve-dependencies/src/index.ts

  Line 187 in d5496cc

  const manifest = updatedOriginalManifest ?? project.originalManifest ?? project.manifest

• pnpm/pkg-manager/core/src/install/index.ts

  Lines 750 to 752 in d5496cc

  	if (project.originalManifest != null) {
  	project.originalManifest = await _removeDeps(project.originalManifest)
  	}

• pnpm/pkg-manager/core/src/install/index.ts

  Line 370 in d5496cc

  manifest: project.originalManifest ?? project.manifest,

[welcome]

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

[Silic0nS0ldier]

Ah. Right. fs.promises.rm is a new API. I'll get that flipped to the backward compatible unlink.

[zkochan]

It doesn't seem right to me.

First of all, why do you write the original manifest at all? If it is the original one, then it is already the one that is saved in the file system.

Secondly, this means that the file will not be updated when opts.save is true. It doesn't make sense.

Maybe what you need to do is changing line 257 to:

if (opts.save !== false && !opts.lockfileOnly) {

[Silic0nS0ldier]

My proposed fix may well be wrong, but I'll add some context around how I got here.

Elsewhere in the repo I noticed .manifest and .originalManifest are both modified to apply changes like removing dependencies and something in @pnpm/resolve-dependencies that I assume handles adding and upgrading.

• Removing, replaces manifest object properties.
  

  pnpm/pkg-manager/core/src/install/index.ts

  Lines 749 to 752 in 54128c9

  	project.manifest = await _removeDeps(project.manifest)
  	if (project.originalManifest != null) {
  	project.originalManifest = await _removeDeps(project.originalManifest)
  	}

• Adding and upgrading (I think), mutates input manifest objects.
  

  pnpm/pkg-manager/resolve-dependencies/src/updateProjectManifest.ts

  Lines 45 to 57 in 54128c9

  	const hookedManifest = await updateProjectManifestObject(
  	importer.rootDir,
  	importer.manifest,
  	specsToUpsert
  	)
  	const originalManifest = (importer.originalManifest != null)
  	? await updateProjectManifestObject(
  	importer.rootDir,
  	importer.originalManifest,
  	specsToUpsert
  	)
  	: undefined
  	return [hookedManifest, originalManifest]

Interpretation as such was "originalManifest must mean the manifest without mutations from the readPackage hook".

[zkochan]

The original manifest is modified, sure. If I remember correctly, original manifest is only not modified by changes done by the hooks.

[zkochan]

actually, you might be right. This is what you had to change. The names of variables are confusing.

[zkochan]

Please add a changeset that describes your changes.

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉

[Silic0nS0ldier]

Thanks for adding the changeset file. I'll keep it in mind for next time.