vercel / turborepo

Build system optimized for JavaScript and TypeScript, written in Rust
https://turbo.build/repo/docs
MIT License
26.34k stars 1.83k forks source link

pnpm's dependenciesMeta.*.injected can't re-sync #2306

Closed NullVoxPopuli closed 8 months ago

NullVoxPopuli commented 2 years ago

What version of Turborepo are you using?

1.6.1

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

pnpm

What operating system are you using?

Linux

Describe the Bug

When using pnpmsdependenciesMeta.*.injectedfeature (to make deep peers work), rebuilding the libraries to get thedistfolder back requires "re-syncing" viapnpm install` (which takes ~1s)

See this related issue on the pnpm repo: https://github.com/pnpm/pnpm/issues/4965#issuecomment-1287926075

Expected Behavior

This is more a bug with pnpm, I think, and I don't know what turbo could do differently -- I'm mostly reporting for SEO / audit-trail purposes (so feel free to close this issue, if there is nothing turbo can do). As a work-around I thought of without turbo would no longer work with turbo -- and that work around is to add pnpm i; as a prefix to every script -- quite obnoxious, and the only real solution is to probably fix the issue in pnpm (which.. I'm not sure even how to approach the problem of re-syncing -- does pnpm need a background daemon that does the re-syncing? I'm not sure).

To Reproduce

I have a sample / repro here: https://github.com/CrowdStrike/ember-headless-table/pull/44 It's gone through a number of iterations, but demonstrates:

weyert commented 2 years ago

I am never really sure when you are supposed to use dependenciesMeta.*.injected. I currently use workspace:* as the version of my internal packages and it appears that turbo correctly build everything in order so it works.

Why do you need to inject the dependency?

NullVoxPopuli commented 2 years ago

You need to inject a library when that library is in your monorepo, and that library declares peers (that are also devDeps) that the consumers of the library is supposed to declare.

Otherwise peers don't resolve correctly -- they resolve to your library's devDeps, instead of the consumer's deps

nathanhammond commented 2 years ago

A possibly better workaround: create a root task called resync, make build: //#resync, ^build, and the mess is contained in one nice neat spot that you can easily delete if pnpm gets clever.

For bonus points this addresses merge-caused node_modules out of sync issues at the same time so maybe not worthwhile to delete.


We're not planning to do anything with this (other than recommend the most-elegant of workarounds) so I will close this. 🤷‍♂️ Thank you for filing the issue; it really helps to see what people are encountering.

nathanhammond commented 2 years ago

After further discussion, I discovered that this is a request to run things at the leaf nodes, not at the root: after we have already acquired all of the information necessary to know if we would need to do this.

We should definitely patch up this papercut; we are actually even better positioned to do this.

weyert commented 2 years ago

Looks like I am having a similar issue now after importing some package heavy project in my monorepo playground.

@NullVoxPopuli Did you had luck with @nathanhammond suggestion?

nathanhammond commented 2 years ago

@weyert My pitch doesn't work (which is why I reopened this).

GiancarlosIO commented 2 years ago

In case someone needs this, there is an issue in pnpm with a reproduction repo that we can use to test solutions: The error is fixed when using the feature injected feature

NullVoxPopuli commented 2 years ago

To be clear though, that fix also causes the behavior reported and linked in the original post in this thread.

NullVoxPopuli commented 1 year ago

@simonihmig came up with a way to make this work over at: https://github.com/CrowdStrike/ember-headless-form/pull/52

It requires this util: https://github.com/CrowdStrike/ember-headless-form/pull/52/files#diff-cba6a6cd1877ea907fa4d3c06bdd8548011bc52b603638c9e8334870e53aca39

does it make sense for turborepo to adopt? or maybe it could be a util for pnpm -- since pnpm is already written in js/ts

NullVoxPopuli commented 1 year ago

Here is a work-around if folks run in to this: https://github.com/NullVoxPopuli/pnpm-sync-dependencies-meta-injected

mehulkar commented 8 months ago

I'm mostly reporting for SEO / audit-trail purposes (so feel free to close this issue, if there is nothing turbo can do)

Closing this out as the related pnpm discussion also suggests that it's not a turborepo-specific problem.

NullVoxPopuli commented 8 months ago

also suggests that it's not a turborepo-specific problem

there is probably a meta issue here (or somewhere) where folks may want to run something that isn't cached before running a regular task (that could be cached).

for example:

turbo build