Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Wrong dependent task gets run when using pnpm relative path reference (eg. workspace:../path/to/dep) #6744

Closed
1 task done
steveluscher opened this issue Dec 8, 2023 · 4 comments · Fixed by #7512
Closed
1 task done

Wrong dependent task gets run when using pnpm relative path reference (eg. workspace:../path/to/dep) #6744

steveluscher opened this issue Dec 8, 2023 · 4 comments · Fixed by #7512
Labels
kind: bug Something isn't working linear: turbo Issues to sync to Linear for Turborepo team owned-by: turborepo

Comments

@steveluscher
Copy link
Contributor

steveluscher commented Dec 8, 2023 •
edited by padmaia
Loading

Verify canary release

  • I verified that the issue exists in the latest Turborepo canary release.

Link to code that reproduces this issue

https://github.com/steveluscher/pnpm-alias-pathspec-repro.git

What package manager are you using / does the bug impact?

pnpm

What operating system are you using?

Mac

Which canary version will you have in your reproduction?

1.11.1-canary.0

Describe the Bug

Imagine that you:

  1. Alias a package to a relative path (eg. "luigi": "workspace:../waluigi")
  2. Another package in your workspace has the same name as the alias (luigi)

…then you run some turborepo task.

pnpm i
cd packages/app/
pnpm turbo hello

Turborepo will run the task in the wrong workspace package; the hello script from luigi/package.json runs.

Expected Behavior

Since the dependency luigi is aliased to workspace:../waluigi, I would expect the hello script from waluigi/package.json to run.

To Reproduce

Follow the README in https://github.com/steveluscher/pnpm-alias-pathspec-repro.git.

Additional context

Note

If you change the dependency in app/package.json from "luigi": "workspace:../waluigi" to "luigi": "workspace:waluigi@*" it works as expected.

TURBO-1899
@steveluscher steveluscher added kind: bug Something isn't working needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels Dec 8, 2023
@steveluscher steveluscher changed the title Wrong dependent task gets run when using pnpm workspace:../path/to/dep reference Wrong dependent task gets run when using pnpm relative path reference (eg. workspace:../path/to/dep) Dec 8, 2023
@steveluscher
Copy link
Contributor Author

Possibly related TODO:

turbo/crates/turborepo-repository/src/package_graph/builder.rs

Lines 624 to 626 in 4be6e0e

// TODO: Since support at the moment is non-existent for workspaces that contain
// multiple versions of the same package name, just assume its a
// match and don't check the range for an exact match.

@chris-olszewski
Copy link
Contributor

I know this isn't a fix for the underlying issue, but could you use the other pnpm alias strategy to unblock yourself?
e.g. in packages/app/package.json change the dependency to be "luigi": "workspace:waluigi@*"

@steveluscher
Copy link
Contributor Author

I mentioned that alternate strategy in a note in the original issue text, yeah. That strategy won't work for me for reasons that I shouldn't get into here. I just want to make sure that Vercel is aware of the bug in Turbo.

@chris-olszewski
Copy link
Contributor

Ah, my bad for not reading the issue closely enough. Thanks for opening the issue and the repro.

@chris-olszewski chris-olszewski added linear: turbo Issues to sync to Linear for Turborepo team and removed needs: triage New issues get this label. Remove it after triage labels Dec 12, 2023
@chris-olszewski chris-olszewski mentioned this issue Feb 23, 2024
Merged
chris-olszewski added a commit that referenced this issue Feb 29, 2024
@chris-olszewski
fix: support pnpm workspace path aliasing (#7512)
85fcacd 
### Description

Fixes #6744 

I highly suggest reviewing each commit on it's own as the code move done
in the first commit makes the cumulative diff hard to understand. I
wanted to clean up this code a bit as we'll need to extend it further
for supporting #7188

This adds support for the PNPM feature of [aliasing a package via
path](https://pnpm.io/workspaces#referencing-workspace-packages-through-their-relative-path).

The slightly tricky part of this is that there are many valid specifiers
for the `workspace:` protocol. We currently support `workspace:1.2.3`,
`workspace:*`,
[`workspace:bar@*`](https://pnpm.io/workspaces#referencing-workspace-packages-through-aliases),
`workspace:../path/to/package`.

### Testing Instructions

Added unit tests for the case of a dependency of the form `"foo":
"workspace:../bar"` where `foo` is another workspace package.

Verified fix works with repro provided in:
#6744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind: bug Something isn't working linear: turbo Issues to sync to Linear for Turborepo team owned-by: turborepo
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: support pnpm workspace path aliasing vercel/turbo
2 participants
@steveluscher @chris-olszewski