Skip to content

Flawed topological sort for subgraph selected with --filter #8335

Description

@adrian-branescu

Verify latest release

  • I verified that the issue exists in the latest pnpm 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?

  • macOS
  • Windows
  • Linux

If your OS is a Linux based, which one it is? (Include the version if relevant)

AlmaLinux

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions