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

[rush] using yarn not always updates node_modules of the module after update #1748

Open
1 of 2 tasks
RIP21 opened this issue Feb 14, 2020 · 21 comments
Open
1 of 2 tasks
Labels
bug Something isn't working as intended repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem

Comments

@RIP21
Copy link

RIP21 commented Feb 14, 2020

Feature or a bug?

  • Feature
  • Bug

Actual behavior.

Simple steps are. You are using yarn: 1.22.0 (same with 1.21.1 for sure)

  1. rush update
  2. Go to one of the packages b in monorepo and add one dependency existent in other packages in monorepo e.g[email protected]
  3. rush update/install
  4. Go to b/node_modules/* looking for node_modules/a
  5. It's not there.

Only rush update --full --purge makes it working which is super annoying and long (6-7 minutes in my case)

Expected behavior.
It's there and stuff works as expected. (Using pnpm it's reliable and works like that)

  • Tool: Rush
  • Tool Version: 5.19.4
  • Node Version: 12.14.1
    • Is this a LTS version? : Yes
    • Have you tested on a LTS version? : Yes
  • OS: MacOS 10.15.3
@RIP21 RIP21 changed the title [rush] yarn not always updates node_modules of the module after update [rush] using yarn not always updates node_modules of the module after update Feb 14, 2020
@RIP21
Copy link
Author

RIP21 commented Feb 21, 2020

Tested on Rush 5.20 the same behavior.
If somebody added a dependency to its package in previous commits, and when you check out these commits you run rush update expecting these new dependencies to propagate to modules node_modules they are not getting propagate until full rush update --purge
Which is super annoying.
Please, give some hints, where to see and how to debug, I may fix it up myself if you guys have no time for that.

@RIP21
Copy link
Author

RIP21 commented Feb 21, 2020

Ok. So what I tell you so far.
It both not working for local modules as well as the ones that are getting installed externally.
rush update with --full or --uncheck nor link and unlink doesn't help with the issue.
Only full rush update --full --purge helps, which is far from optimal and takes 7-9 minutes in our project.

What I can see after I tried all that above is that updates are not getting populated.

In the projects folder in the package.json's and in tar.gz files all list of dependencies are actually updated.

The problem is with rush-link.json that still consists of an old set of local modules. And even deletion and unlink and link doesn't fixes this.
What about 3rd parties, I think the issue lies in yarn.lock file that still says that module B doesn't have this new dependency in its list of dependencies.

To put it in an example:

Let's say we add an external package EXT-A and internal package A to package B that has EXT-B and internal C as a dependency already.
So rush-link.json looks something like before update

"B": [
  "C"
]

After we run rush update it should become

"B": [
  "C",
  "A"
]

Since we added an internal package A to him.
Same applies to its yarn.lock record.
From

"@rush-temp/B@file:./projects/B.tgz":
  version "0.0.0"
  resolved "file:./B#512633d60d5549cdcb75cf0594cdba4d300aa7d8"
  dependencies:
    "EXT-B" "1.0.0"

It should become

"@rush-temp/B@file:./projects/B.tgz":
  version "0.0.0"
  resolved "file:./B#512633d60d5549cdcb75cf0594cdba4d300aa7d8"
  dependencies:
    "EXT-B" "1.0.0"
    "EXT-A" "1.0.0"

Because we added external EXT-A dependency.

Problem is that nothing of that happens, so B still, after rush update or rush update --full stays with old set of dependencies in the node_modules.
(I may be wrong, and 3rd parties after --full actually added, but it's not reflected in lock file, but an internal package is not linked 100%)

@RIP21
Copy link
Author

RIP21 commented Feb 21, 2020

And, by the way, I have 2 allowed packages with 2 versions each that are allowed. If that may change anything in all this algorithm and actually break this thingy.

@RIP21
Copy link
Author

RIP21 commented Feb 21, 2020

@octogonz some guidance where to look will help :) Or maybe it's me who did something wrong, but I don't think so. (I'll try to make repro repo later today if will have some time)

@octogonz
Copy link
Collaborator

Yes, that would be super helpful. I have a lot on my plate right now, but if you can provide an isolated repro, I'm willing to at least investigate it and explain how it could be fixed. We might need to get someone else to create the PR. Thanks @RIP21 !

@RIP21
Copy link
Author

RIP21 commented Feb 21, 2020

@octogonz here we go!
There is a bee nest of issues out there that I have found :D I think there are even more issues than I thought (some are totally crazy). Probably they all come from one source, but hopefully, this will help.
All steps to reproduce are included in README.md

Can't wait for fix!
Here we go :)
https://github.com/RIP21/yarn-rush-issue-repro

@RIP21
Copy link
Author

RIP21 commented Feb 24, 2020

@octogonz
If it can be prioritized I'll super super appreciate :( Struggle is real, especially mine, explaining to people what is broken and how to work around it.

It's really a deal-breaker for us, and we can't move to npm 4.5 cause of missing peer dependencies that we can't resolve that make NPM run of npm shrinkwrap fail.

pnpm is a solution, but it will take a week or two to migrate.

Also, rush update --purge not always helps, only rush update --purge --full the most reliable, but it causes some ranged versions inside 3rd party packages to bump breaking stuff sometimes. (we have strict concrete versions everywhere in our code but not 3rd party)

Thanks, and sorry for chase :)

@apostolisms
Copy link
Contributor

@RIP21 is this something you observed when you updated rush's version or was that always an issue ?

@RIP21
Copy link
Author

RIP21 commented Feb 24, 2020

@apostolisms it was always like that. But we migrated exactly a week ago, so the only version I tried is 5.19 and 5.20. I tried some old versions of yarn too, just to be on the safe side, same results.

@RIP21
Copy link
Author

RIP21 commented Feb 24, 2020

I'll be super happy if there is older version that is stable with yarn updates, but fix of the latest would be the best :)

@RIP21
Copy link
Author

RIP21 commented Mar 16, 2020

@octogonz I have added repro repository more than 3 weeks ago. Please check it out. (adding repro repo added label will be nice too :P )

@octogonz octogonz added repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem bug Something isn't working as intended labels Mar 26, 2020
@octogonz
Copy link
Collaborator

octogonz commented Mar 26, 2020

@RIP21 Thank you for providing this repro! Sorry it took a while to respond. (Things are a little crazy right now!)

I took a look and confirmed that it is a bug. Let me see what our options are and then follow up.

@RIP21
Copy link
Author

RIP21 commented Mar 26, 2020 via email

@octogonz
Copy link
Collaborator

PNPM is an excellent package manager. 👍

@RDeluxe
Copy link

RDeluxe commented Apr 28, 2020

Hello, seems that #1836 is a duplicate of this issue.

In our case we can't migrate to PNPM because mediasoup would not build.

@RIP21
Copy link
Author

RIP21 commented Apr 28, 2020

Hello, seems that #1836 is a duplicate of this issue.

In our case we can't migrate to PNPM because mediasoup would not build.

Yes it's a duplicate. I don't know why it's ignored for that long. I did everything I could so it gets fixed. Decent repro with steps and stuff.
Until this fixed yarn support shouldn't be advertised at all because it just doesn't work properly. We didn't had another chance but to migrate to pnpm, which also very rarely has issues with rush, but only latest versions, pnpm 4.9.* I think it's what we use and it's rock solid so far.

@RIP21
Copy link
Author

RIP21 commented Jun 3, 2020

@octogonz this one still needs to be fixed :( Or just remove the copy about yarn support from the docs if this one is not a priority and never going to be fixed.
It's already 5 months since it's opened and no action since then.

@octogonz
Copy link
Collaborator

octogonz commented Jun 4, 2020

We're working towards this plan that will make Yarn much easier to support with Rush: #1553

The first big step is in this recent PR: #1897

We do want to improve the support for both Yarn and NPM, but the Rush maintainers pretty much exclusively use PNPM, so it has been difficult to prioritize. If someone from the community wants to help out, that would speed things up. (But we will do this work either way, because there are certain important scenarios that still have compatibility issues with PNPM.)

@RIP21
Copy link
Author

RIP21 commented Jun 4, 2020

@octogonz thanks for update. My concern is that there is more and more cases for monorepos in my company, but everywhere we have yarn, so it's complicates things when moving towards rush (which I think the best monorepo manager out there for existing Node projects)
But since it's not a huge priority yet (like it used to be for one project), I won't be able to offer help right now. If it will become urgent and here it will be smaller effort than adopt pnpm internally I'll definitely come offering help once again :)
Other than that I hope it will be fixed along the other improvements you do.

dbartholomae added a commit to dbartholomae/lambda-middleware that referenced this issue Aug 13, 2020
@PrimarchAlpharius
Copy link

Any updates on this?

@amir-saadallah
Copy link

the issue still there impossible to work with yarn

@iclanton iclanton moved this to Needs Investigation in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem
Projects
Status: Needs Investigation
Development

No branches or pull requests

6 participants