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.

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

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

Closed
NullVoxPopuli opened this issue Oct 22, 2022 · 12 comments
Closed

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

NullVoxPopuli opened this issue Oct 22, 2022 · 12 comments

Comments

@NullVoxPopuli
Copy link

NullVoxPopuli commented Oct 22, 2022

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: pnpm/pnpm#4965 (comment)

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: CrowdStrike/ember-headless-table#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
@weyert
Copy link
Contributor

weyert commented Oct 23, 2022

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
Copy link
Author

NullVoxPopuli commented Oct 23, 2022

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
Copy link
Contributor

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.

NullVoxPopuli added a commit to CrowdStrike/ember-headless-table that referenced this issue Oct 23, 2022
…ms like resync finishes too early, and we need it to run specifically after the library build
@nathanhammond
Copy link
Contributor

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 nathanhammond reopened this Oct 24, 2022
@weyert
Copy link
Contributor

weyert commented Oct 25, 2022

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
Copy link
Contributor

nathanhammond commented Oct 26, 2022

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

@GiancarlosIO
Copy link

GiancarlosIO commented Oct 28, 2022

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
Copy link
Author

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

NullVoxPopuli added a commit to CrowdStrike/ember-headless-table that referenced this issue Jan 10, 2023
This is a squashed commit, for easier rebasing and conflict resolution.
Previous topics in now hidden commits:

chore(turbo): try suggestion from: vercel/turbo#2306 (comment) -- seems like resync finishes too early, and we need it to run specifically after the library build
chore(turbo, ci): setup turbo before pnpm install
chore(turbo): try cache-hit
chore(scripts): tidy up the start meta-script
NullVoxPopuli added a commit to CrowdStrike/ember-headless-table that referenced this issue Jan 10, 2023
This is a squashed commit, for easier rebasing and conflict resolution.
Previous topics in now hidden commits:

chore(turbo): try suggestion from: vercel/turbo#2306 (comment) -- seems like resync finishes too early, and we need it to run specifically after the library build
chore(turbo, ci): setup turbo before pnpm install
chore(turbo): try cache-hit
chore(scripts): tidy up the start meta-script
@NullVoxPopuli
Copy link
Author

@simonihmig came up with a way to make this work over at: CrowdStrike/ember-headless-form#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
Copy link
Author

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

@mehulkar mehulkar added needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels Oct 20, 2023
@mehulkar
Copy link
Contributor

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 mehulkar removed the needs: triage New issues get this label. Remove it after triage label Mar 11, 2024
@NullVoxPopuli
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants