turbo icon indicating copy to clipboard operation
turbo copied to clipboard

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

Open NullVoxPopuli opened this issue 2 years ago • 8 comments

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 pnpms dependenciesMeta.*.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:

  • cache working
  • but injected dependency is not detected by downsteam projects in the monorepo

NullVoxPopuli avatar Oct 22 '22 21:10 NullVoxPopuli

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?

weyert avatar Oct 23 '22 01:10 weyert

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

NullVoxPopuli avatar Oct 23 '22 01:10 NullVoxPopuli

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 avatar Oct 23 '22 15:10 nathanhammond

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.

nathanhammond avatar Oct 23 '22 15:10 nathanhammond

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?

weyert avatar Oct 25 '22 23:10 weyert

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

nathanhammond avatar Oct 26 '22 10:10 nathanhammond

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

  • https://github.com/pnpm/pnpm/issues/5479

giancarlosisasi avatar Oct 28 '22 16:10 giancarlosisasi

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

NullVoxPopuli avatar Oct 28 '22 16:10 NullVoxPopuli

@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 avatar Feb 16 '23 18:02 NullVoxPopuli

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

NullVoxPopuli avatar Jun 22 '23 16:06 NullVoxPopuli

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.

mehulkar avatar Mar 11 '24 19:03 mehulkar

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

  • for each workspace
    • run: build
      • regardless of cache hit/miss: run "X" after "build" is finished -- in this case, it'd be the sync util

NullVoxPopuli avatar Mar 11 '24 19:03 NullVoxPopuli