In a workspace, also include missing deeply linked workspace packages at headless installation

[kenrick95]

Attempt to fix #5034

Problem:

When using link-workspace-packages = deep + filter, there could be case where it depends on a project inside the workspace that is not selected by the filter. We only know what are the projects after we resolve the dependencies.

Idea for solution:

"resolve-dependencies", after the first "resolveDependencyTree", when we encounter projects in "dependenciesTree" that are not yet part of the "projectsToResolve", we add it into "importers" & "projectsToResolve" and run "resolveDependencyTree" again and again till all projects in "dependenciesTree" are part of "projectsToResolve"

I wonder if the solution sounds ok?

Not fully working yet, have some questions (inline in codes)

plus there are some failing tests (test/linkRecursive.ts & test/miscRecursive.ts) due to this change

Updated solution:

• core passes allProjects to headless
• filterByImportersAndEngine finds out the missing deeply linked projects while traversing the lockfile
• headless updates "opts.options" with missing projects

[zkochan]

TBH, I don't think running pnpm install with any filters is a good idea when the lockfile needs changes. I think we probably only need to allow filtered installation with an up-to-date lockfile (especially as I want to implement a new resolution mode). Or alternatively, we should resolve all the dependencies of all the packages but then only link those projects that were filtered. Though the issue with the second approach is that we currently fetch the package to resolve it (related issue: #1328)

[kenrick95]

I think we probably only need to allow filtered installation with an up-to-date lockfile

yeah I agree on this part

we should resolve all the dependencies of all the packages but then only link those projects that were filtered

I feel that this wouldn't address the issue #5034 😅

[zkochan]

I feel that this wouldn't address the issue #5034 sweat_smile

The issue should be fixed for the up-to-date lockfile only. That means that the fix needs to be in the headless package.

[zkochan]

If we pass all the projects to core anyway, maybe we should do the project filtering inside core and headless. That might improve the code quality.

[kenrick95]

maybe we should do the project filtering inside core and headless

yea that makes sense 🤔

[kenrick95]

maybe we should do the project filtering inside core and headless

I attempted this... It feels like I would need to move and split whole plugin-installation's recursive.ts to core and headless, but I'm not confident in doing so..

[zkochan]

I'll look into it.

[kenrick95]

Hi @zkochan, is the refactor a blocker to this PR?

[zkochan]

yes

[zkochan]

I'll review it. Maybe it is ok.

[zkochan]

ok, I think I know the right way to do it. Project filtering should not be moved. However, some refactoring is needed. Currently you pass all the projects to @pnpm/core. The same objects are also duplicated inside MutatedProject. And then you read the context of projects twice. Once in @pnpm/core and once in @pnpm/get-context.

What we should do instead is removing the manifest fields from MutateProject. All the info is already present in the new allProjects field. So we just need to make this new field required and read the data from it.

It might be hard to implement. I can probably work on this refactoring if you don't want to.

[kenrick95]

Thanks for the feedback. Unfortunately I'm not very confident in doing that. Please do help 🙏 🙏

[zkochan]

The refactor is in progress in this PR: #5338

[zkochan]

@kenrick95 I have merged the refactoring that had to happen before these changes. Now headless install receives the allProjects option where you can get all the info that you need.