Verify latest release
pnpm version
9.3.0 & 9.6.0
Which area(s) of pnpm are affected? (leave empty if unsure)
Dependencies resolver, CLI
Link to the code that reproduces this issue or a replay of the bug
https://github.com/adrian-branescu/pnpm-flawed-topo
Reproduction steps
As explained in the readme of the reproduction repo:
You have the following workspaces:
a depends on b
b depends on c
c has no dependencies
Each of them has a build NPM script.
Run in monorepo root the following command:
pnpm run \
--filter a \
--filter c \
build
You will get the following output:
Scope: 2 of 4 workspace projects
a build$ echo 'Building a ...'
│ Building a ...
└─ Done in 21ms
c build$ echo 'Building c ...'
│ Building c ...
└─ Done in 22ms
As you can observe, a is built before or at best concurrently with c, although c is a transitive dependecy of a.
Describe the Bug
The topological sort is flawed when --filter is used to select a subset of workspaces that depend indirectly on each other.
Reason
You are sorting the subgraph obtained after applying --filter, but this sort is useless since you've lost the edges between the nodes not selected in this subgraph. These edges may describe transitive dependency relationships between the selected nodes.
Line of code:
https://github.com/pnpm/pnpm/blob/v9.6.0/exec/plugin-commands-script-runners/src/runRecursive.ts#L53
Expected Behavior
The topological sort of the subgraph selected with --filter should be correct even for transitive dependencies. In our case, c has to be built before a.
Proposed solution
You could perform the sort before applying --filter aka sort the graph with all workspaces and then preserve only those workspaces that are matching --filter.
Line of code:
https://github.com/pnpm/pnpm/blob/v9.6.0/exec/plugin-commands-script-runners/src/runRecursive.ts#L53
Could become something like this:
const sortedAllPackageChunks = sortPackages(opts.allProjectsGraph);
const orderedPackages = sortedAllPackageChunks.map((prefixes, buildIndex) => prefixes.map((rootDir) => ({
buildIndex,
rootDir
}))).flat();
const filtered = orderedPackages.filter(pkg => Object.keys(opts.selectedProjectsGraph).includes(pkg.rootDir));
Which Node.js version are you using?
20.15.0
Which operating systems have you used?
If your OS is a Linux based, which one it is? (Include the version if relevant)
AlmaLinux
Verify latest release
pnpm version
9.3.0 & 9.6.0
Which area(s) of pnpm are affected? (leave empty if unsure)
Dependencies resolver, CLI
Link to the code that reproduces this issue or a replay of the bug
https://github.com/adrian-branescu/pnpm-flawed-topo
Reproduction steps
As explained in the readme of the reproduction repo:
You have the following workspaces:
adepends onbbdepends oncchas no dependenciesEach of them has a
buildNPM script.Run in monorepo root the following command:
pnpm run \ --filter a \ --filter c \ buildYou will get the following output:
As you can observe,
ais built before or at best concurrently withc, althoughcis a transitive dependecy ofa.Describe the Bug
The topological sort is flawed when
--filteris used to select a subset of workspaces that depend indirectly on each other.Reason
You are sorting the subgraph obtained after applying
--filter, but this sort is useless since you've lost the edges between the nodes not selected in this subgraph. These edges may describe transitive dependency relationships between the selected nodes.Line of code:
https://github.com/pnpm/pnpm/blob/v9.6.0/exec/plugin-commands-script-runners/src/runRecursive.ts#L53
Expected Behavior
The topological sort of the subgraph selected with
--filtershould be correct even for transitive dependencies. In our case,chas to be built beforea.Proposed solution
You could perform the sort before applying
--filteraka sort the graph with all workspaces and then preserve only those workspaces that are matching--filter.Line of code:
https://github.com/pnpm/pnpm/blob/v9.6.0/exec/plugin-commands-script-runners/src/runRecursive.ts#L53
Could become something like this:
Which Node.js version are you using?
20.15.0
Which operating systems have you used?
If your OS is a Linux based, which one it is? (Include the version if relevant)
AlmaLinux