microsoft / rushstack

Monorepo for tools developed by the Rush Stack community
https://rushstack.io/
Other
5.82k stars 592 forks source link

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

Open RIP21 opened 4 years ago

RIP21 commented 4 years ago

Feature or a 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.ga@1.0.0
  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)

RIP21 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

RIP21 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@octogonz Thank you :) Even who we migrated to pnpm already because it was the easiest solution for us. For other projects that will be super helpful.

чт, 26 мар. 2020 г. в 23:55, Pete Gonzalez notifications@github.com:

@RIP21 https://github.com/RIP21 Thank you for providing this repro! I took a look and confirmed that it is a bug. Let me see what our options are and then follow up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/microsoft/rushstack/issues/1748#issuecomment-604728754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6B532NQMM3EJU7CTGQNI3RJPMPRANCNFSM4KVK226A .

-- Andrii Los. Senior Software Engineer at Revolut.com

http://goog_1948405826 Github https://github.com/RIP21

octogonz commented 4 years ago

PNPM is an excellent package manager. 👍

RDeluxe commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

We're working towards this plan that will make Yarn much easier to support with Rush: https://github.com/microsoft/rushstack/issues/1553

The first big step is in this recent PR: https://github.com/microsoft/rushstack/pull/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 commented 4 years ago

@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.

PrimarchAlpharius commented 2 years ago

Any updates on this?

amir-saadallah commented 1 year ago

the issue still there impossible to work with yarn