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

yarn dedupes inconsistently #6070

Open
TheLudd opened this issue Jul 7, 2018 · 5 comments
Open

yarn dedupes inconsistently #6070

TheLudd opened this issue Jul 7, 2018 · 5 comments

Comments

@TheLudd
Copy link

TheLudd commented Jul 7, 2018

After a discusion on discord with @rally25rs earlier this week I was advised to create an issue here for a discussion on how yarn handles deduping.

First, running yarn dedupe produces this output which is also available here:

error The dedupe command isn't necessary. yarn install will already dedupe.

However, yarn install will infact not dedupe if you have a duplicated structure. Duplicated structures are created today when upgrading specific packages. The flow is usually something like this:

  • Your project depends on react
  • It also depend on a react module that itself depends on react
  • Your project upgrades react, and even though the new react version is in the range of the other modules specified react dependency, react is duplicated in the node_modules tree.

This is a problem because you can get duplicate instances of react which is not valid. This is just an example however, there are other packages than react, such as our own internal modules at my company, where duplicated modules cause problems.

The only way now to deduplicate is to run a general yarn upgrade which has the side effect of upgrading all other dependencies as well.

I am not sure if the current behavior is intended or not but IMO yarn should always deduplicate as much as possible. If I remember correctly, this was a goal of yarn when it was initially launched.

Could yarn be implemented to dedupe on upgrades?

@ghost ghost assigned rally25rs Jul 7, 2018
@ghost ghost added the triaged label Jul 7, 2018
@rally25rs
Copy link
Contributor

rally25rs commented Jul 7, 2018

@yarnpkg/core looking for some discussion on the correct behavior here...

Some of my notes from our discord discussion:

if multiple patterns resolve to the same version you'll get more than 1 key to the same value
so in your example, if the root project depends on a@^1.0.0 and b depends on a@^1.0.1 (in other words the dep tree is:

package.json
  |- a@^1.0.0
  +- b
      +- a@^1.0.1

In the lockfile you get something like:

[a@^1.0.0, a@^1.0.1] -> [email protected]

assuming that [email protected] is latest at the time it was added.

if you yarn upgrade a because [email protected] exists then it unlocks the root's dependency on a (removes a@^1.0.0) and leaves you with

[a@^1.0.1] -> [email protected]

then re-adds the latest a@^1.0.0 which now resolves to [email protected] so the lockfile becomes

[a@^1.0.1] -> [email protected]
[a@^1.0.0] -> [email protected]

(not saying that's "correct", just saying I'm pretty sure that's how the code currently works)


@TheLudd suggests that after yarn upgrade a yarn should dedupe a@^1.0.0 and [email protected] to both point to [email protected] which is the max version satisfying both ranges.

My feedback was:

It's possible that if you had

if running yarn upgrade b changed it to

some people might find that to be a bug (yarn changed a's dependencies but I didn't upgrade a) ... but I could certainly see an argument either way.

My concern is that I think the behavior might actually be a bit ... undefined at the moment.
If they used the same pattern then I think the above change would happen, but if they used different patterns then it would not change a -> b and become

so whether or not a -> b changes or not is unclear... It's be nice to get some more opinions on what's "correct" in these cases and make sure it at least behaves in a consistent way.


What I'm talking about with the "inconsistent" behavior is that if the dep tree used the same ranges from the original example:

package.json
  |- a@^1.0.0
  +- b
      +- a@^1.0.0

In the lockfile you get:

[a@^1.0.0] -> [email protected]

then yarn upgrade a will also change the version of a that b is using, because it is using a hoisted and deduped version of a

@bestander
Copy link
Member

bestander commented Jul 16, 2018

I totally agree.
Let's do it!
I mean the upgrade command to scan the tree and upgrade in other places.

@epmatsw
Copy link

epmatsw commented Jul 26, 2018

Just want to add another vote that the deduplication approach is the better one. The current state can make it difficult/impossible to upgrade a dependency without unlocking the full dependency tree or manually editing the lockfile, which is a big ask when you only want to upgrade a single package.

@epmatsw
Copy link

epmatsw commented Jul 27, 2018

I also think this conversation is discussing a solution for #3967

@Bnaya
Copy link

Bnaya commented Sep 2, 2018

I'm using
https://www.npmjs.com/package/yarn-tools
fix-duplicates
And it really works great.

Jessidhia referenced this issue in DefinitelyTyped/DefinitelyTyped Nov 12, 2018
* Add definitions for React.memo

* Add missing semicolons

* Give a better name to the second argument to React.memo

* Fix test to reflect correct usage of React.memo's second argument

* Fix no-unnecessary-qualifier lints

* Update other special components to be SpecialSFC

* Ensure ordinary functions aren't assignable to SpecialSFC

* createElement was resolving P to "{} | null" in some tests; add extends to prevent that

* Fix the props of Fragment and StrictMode

* Add tests for SpecialSFC assignability

* Add scary doc comment to SpecialSFC's call signature

Hopefully this trips tslint's deprecation rule.

* Rename SpecialSFC to ExoticComponent

* Add overload to React.memo to make it more ergonomic and avoid implicit any

* Disable test that requires TS 3.1

* Add support for displayName in the exotic components that support it

* Tone down the call signature's doc comment

* Correct $ExpectType assertion

* Try to implement LibraryManagedAttributes for the ExoticComponents that should support them

This doesn't actually work because it seems only class components get them checked?

* Add type definitions for new React 16.6 features

This also attempts to add support for LibraryManagedAttributes to the ExoticComponents.

* The fallback prop is required by Suspense

* Declare legacy context on tests that use legacy context
mbenadda added a commit to openfun/richie that referenced this issue Nov 13, 2018
There is currently an issue in yarn where it fails to deduplicate
dependencies versions on `yarn install`. See this issue:
yarnpkg/yarn#6070.

This, coupled with a change in the definition of Component in
@types/react, (DefinitelyTyped/DefinitelyTyped@9039892)
breaks us as it forces us to use incompatible versions of @types/react.

We can easily solve this by unlocking all our dependencies (by
deleting node_modules & yarn.lock) and regenerating the lockfile
from scratch. This is stable as it does not create a different
lockfile than what yarn does on its own.
mbenadda added a commit to openfun/richie that referenced this issue Nov 13, 2018
There is currently an issue in yarn where it fails to deduplicate
dependencies versions on `yarn install`. See this issue:
yarnpkg/yarn#6070.

This, coupled with a change in the definition of Component in
@types/react, (DefinitelyTyped/DefinitelyTyped@9039892)
breaks us as it forces us to use incompatible versions of @types/react.

We can easily solve this by unlocking all our dependencies (by
deleting node_modules & yarn.lock) and regenerating the lockfile
from scratch. This is stable as it does not create a different
lockfile than what yarn does on its own.

gitlint-ignore: line-length
mbenadda added a commit to openfun/richie that referenced this issue Nov 13, 2018
There is currently an issue in yarn where it fails to deduplicate
dependencies versions on `yarn install`. See this issue:
yarnpkg/yarn#6070.

This, coupled with a change in the definition of Component in
@types/react, (DefinitelyTyped/DefinitelyTyped@9039892)
breaks us as it forces us to use incompatible versions of @types/react.

We can easily solve this by unlocking all our dependencies (by
deleting node_modules & yarn.lock) and regenerating the lockfile
from scratch. This is stable as it does not create a different
lockfile than what yarn does on its own.

gitlint-ignore: body-max-line-length
mbenadda added a commit to openfun/richie that referenced this issue Nov 15, 2018
There is currently an issue in yarn where it fails to deduplicate
dependencies versions on `yarn install`. See this issue:
yarnpkg/yarn#6070.

This, coupled with a change in the definition of Component in
@types/react, (DefinitelyTyped/DefinitelyTyped@9039892)
breaks us as it forces us to use incompatible versions of @types/react.

We can easily solve this by unlocking all our dependencies (by
deleting node_modules & yarn.lock) and regenerating the lockfile
from scratch. This is stable as it does not create a different
lockfile than what yarn does on its own.

gitlint-ignore: body-max-line-length
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