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 is not deduping certain packages with * dependencies #6695

Open
nickpape opened this issue Nov 16, 2018 · 34 comments
Open

Yarn is not deduping certain packages with * dependencies #6695

nickpape opened this issue Nov 16, 2018 · 34 comments
Assignees
Labels

Comments

@nickpape
Copy link

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Yarn doesn't dedupe @types packages or packages with * range dependencies.

If the current behavior is a bug, please provide the steps to reproduce.

https://github.com/nickpape-msft/yarn-react-types

Go in there and run yarn install. You would expect there to be only the following versions:

"@types/react": "16.4.2",
"@types/react-dom": "16.0.5"

You would expect this because the @types/react that is installed at the root should satisfy the dependency listed in @types/react-dom:

{
    "name": "@types/react-dom",
    "version": "16.0.5",
    "dependencies": {
        "@types/react": "*",
        "@types/node": "*"
    }
}

However, you instead get a side-by-side @types/react dependency:

λ yarn why @types/react
yarn why v1.12.3
[1/4] Why do we have the module "@types/react"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@types/[email protected]"
info Has been hoisted to "@types/react"
info This module exists because it's specified in "dependencies".
=> Found "@types/react-dom#@types/[email protected]"
info This module exists because "@types#react-dom" depends on it.
Done in 0.09s.
λ yarn why @types/react-dom
yarn why v1.12.3
[1/4] Why do we have the module "@types/react-dom"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@types/[email protected]"
info Has been hoisted to "@types/react-dom"
info This module exists because it's specified in "dependencies".
Done in 0.09s.

This would cause you to get TypeScript build errors (the newest types are perhaps using new compiler features, or you end up with duplicate definitions for stuff that is exported from @types/react).

What is the expected behavior?

Only the following two packages are installed:

"@types/react": "16.4.2",
"@types/react-dom": "16.0.5"

Please mention your node.js, yarn and operating system version.

Node: 8.9.4
Yarn: 1.12.3
Windows 10

@nickpape
Copy link
Author

This is causing an issue for some people using our Yeoman generator. As a workaround, we are using resolutions, but since this behavior seems different from other major package managers (and seems to be fundamentally at odds with the @types system, as well as the concept of deduplication), it seemed like a Yarn bug.

SharePoint/sp-dev-docs#2934

@arcanis
Copy link
Member

arcanis commented Nov 19, 2018

Cf this thread: #3951 (comment)

Yes and no. This is a bit tricky.

  • Yes, using * means that the hoister is indeed allowed to consider that the dependency can be safely merged with any other version if it wants to

  • However, it doesn't mean that the hoister is required to do so. Semantically speaking, a dependencies entry literally means "this package MUST be able to require a package of version $VERSION". There's absolutely no mention about this version being compatible with any other. If you need this behavior, then you have to use peerDependencies, which provide the extra guarantee that the selected version MUST be the same as the one provided by the parent package (which is exactly what is asked for in the first post).

So, to sum up: the hoister is about trimming the fat and avoiding filling your disk with garbage - not ensure your dependencies are correctly setup (that's the responsibility of package authors). Ideally, your code should work with no hoisting, or with super-weird hoisting that would create ten levels of indirection. If you don't, it will eventually break, whether it's in Yarn or in another package manager.

Of course, we could potentially fix this (most likely not without spending some serious time, and breaking other behaviors). But I fear that by doing so, we would actually enable this kind of "We can do whatever we want, the package managers will have to make it work anyway" behavior, which I think is detrimental to everyone in the long term (Yarn, but also other package managers who would have to have the exact same behavior, and package consumers that would have to use sub-par package managers that can't apply aggressive optimizations because some packages do not play by the rules - we've already been hit by that in a few occasions). 😉


Now, from an implementation viewpoint, here is what causes the behavior described in the OP. First, you have to understand that what we called the "hoister" is actually splitted in two different parts:

  • The optimizer currently runs during resolution, and tries to avoid resolving ranges that have already been satisfied by previous resolution requests, but the requests are mostly independants (so for example, in the OP situation, when we resolve angular, we have no idea that a sub-dependency has also a dependency on angular, and conversely). The result of the optimizer is what is stored inside the lockfile.

  • The hoister runs once the resolution (and optimization) has completed, and I think simply tries to merge together packages that share a exact same version, without any consideration for their original range.

So during the first step, both angular versions are resolved to their respective versions, independently from each other, then the hoister tries to merge them, see that their versions don't match, and just doesn't merge them. And as described in the first part of this comment, this is to be expected. If you want them to be merged, you would instead use a peerDependencies entry, which would not be resolved and wouldn't suffer the issue described here.

@arcanis
Copy link
Member

arcanis commented Nov 19, 2018

tldr: open a PR on DefinitelyTyped to change the dependencies into peer dependencies 🙂

@octogonz
Copy link

tldr: open a PR on DefinitelyTyped to change the dependencies into peer dependencies

Also, everybody should have a unicorn!

If you can't obtain a unicorn for some reason (e.g. you need one of the 5000+ NPM packages that are not part of this awesome fantasy), then you should use a different package manager than Yarn.

Ermmm... wait a sec-- we want people to use Yarn, right?! 🙂

@milesj
Copy link

milesj commented Nov 28, 2018

Filed a similar issue here: #6415

@arcanis
Copy link
Member

arcanis commented Nov 28, 2018

Also, everybody should have a unicorn!

I want to think that opening a PR to remove an undefined behavior from a package should be less work than finding a unicorn. But someone has to do it at some point, I guess 🙂

@octogonz
Copy link

But (as far as I know) the current definitions work correctly today with NPM and PNPM, which have two radically different installation strategies. So, in what sense is the behavior "undefined"?

Yarn's interpretation seems to be that if I ask for "~1.2.3", I should always get the absolute latest 1.2.99, even if all the other libraries in my locus asked for "1.2.4". That is wasteful, because we have to install, load, and eval two entire copies of the library. It seems pretty reasonable for the package manager to say "1.2.4 is compatible with what you asked for, so it's more economical to give you that."

@zkochan

@octogonz
Copy link

To summarize your request:

  1. Basically all @types dependencies for all 5000+ packages from DefinitelyTyped should be converted to be peer dependencies
  2. (From another thread), each such package must also declare peer dependencies for the transitive closure of all its @types indirect dependencies
  3. Package managers better get ready to process hundreds of peer dependency relationships during installation. (For example, today NPM prints tons of unintelligible logs if the peers fail to install, and acts as if npm install was successful.)

Is that the work needed to make the @types packages compatible with Yarn?

@arcanis
Copy link
Member

arcanis commented Nov 28, 2018

So, in what sense is the behavior "undefined" ?

Two reasons:

  • You have to understand that perfect non-wasteful resolution across a tree is Np-complete, and would require to preemptively download the metadata of all the dependencies of all the releases of all the packages that could possibly end up in the tree. For *, that would include the dependencies of 0.0.1 as well as those of 42.8.2. We (and that includes pnpm and npm) simply cannot guarantee that all the resolutions will be "perfectly optimized" across the tree, so while it seems possible to do a better job at it, it's not something you should rely on. If you need to use the same version than your parent, use a peer dependency.

  • The definition of "perfect" and "non-wasteful" is entirely subjective and whatever implementation we have, someone will complain. You're arguing that * means "anything", and that resolving to old release to optimize for space is "better". I guarantee you that someone else will open an issue if this behavior changes and tell us how "dangerous" it is that we don't resolve to latest every time we can because bugfixes are not automatically pulled etc etc. And I'll have to make the same case again.

When I say undefined behavior, I mean it in the C/C++ sense: it's undefined in the sense that nothing defines what happens when two packages from the dependency tree overlap[1]. They might be resolved to the same version, but maybe not. And the behavior can change if we need to.

[1] Except in Plug'n'Play, where it is standardized that regardless of the position of the dependent package in the dependency tree, the exact same range absolutely must resolve to the exact same version (we can afford to do it there because we're not restricted by the hoisting anymore). Still not guarantees on overlapping-but-different ranges, though (because of the two points I detailed above).

Is that the work needed to make the @types packages compatible with Yarn?

You can work incrementally and fix such packages as they cause issues, but generally yes. You also can send a PR to Yarn to fix the bug that duplicates the resolution, and keep the DefinitelyTyped packages as they are (broken). They'll cause other issues down the road because the core problem won't have been addressed, of course.

If you want a hotfix, the resolution field is the best answer. Just tell Yarn that all versions of your @types package must absolutely resolve to a version of your choice.

@octogonz
Copy link

octogonz commented Nov 28, 2018

Thinking about this more, perhaps peer dependencies are appropriate. As I understand it, the rules for @types packages are:

  • SemVer ranges are useless, because by design the version number communicates the corresponding non-TypeScript package version, and breaking TypeScript syntax changes often happen in a PATCH bump
  • When @types packages depend on each other, finding a compatible set of versions is going to be a Sudoku puzzle
  • The person in the best position to solve the puzzle is the root project being built, so @types packages must always specify "*" SemVer range for their dependencies
  • The root project must always specify exact version numbers (never SemVer ranges), or it will get broken

This really does sound like what peer dependencies are supposed to model.

You can work incrementally and fix such packages as they cause issues, but generally yes.

I believe DefinitelyTyped builds and publishes all 5000+ @types packages as part of a unified process. If we can convince them, they could probably apply this change globally.

@milesj
Copy link

milesj commented Nov 28, 2018

@arcanis Correct me if I'm wrong, but doesn't * mean any? So why would you need to preemptively download all the past versions as well? Wouldn't package@latest be enough? Or simply, whatever the tree has resolved to be the "latest"?

@arcanis
Copy link
Member

arcanis commented Dec 5, 2018

So why would you need to preemptively download all the past versions as well?

Because a "perfect" tree (given any subjective metric) doesn't necessarily use all the latest versions.

For example a "perfect" tree (I hate this term so much) where the project depends on [email protected] and B@* would maybe choose the older release if given the choice between [email protected] (which itself depends on [email protected]) and [email protected] (which itself depends on [email protected]), because it would only require to install two packages vs three with the more recent package.

@mikl
Copy link

mikl commented Mar 6, 2019

The most confusing part of this problem is that it behaves non-deterministically. I had been using Yarn for a TypeScript project for months, and I had been lucky to not hit this problem, until one random afternoon it blows up in your face and starts installing mutually incompatible versions all over the place.

If Yarn is not able to handle"*" versions the way npm does, it should at least warn the user that this syntax is not supported they way the npm ecosystem expects.

The way things stand at the moment, Yarn cannot safely be used in combination with TypeScript (since almost any TS project is bound to use @types/ packages, and the vast majority of them have "*" versions in their dependencies). Sooner or later, this is going to lead to strange compiler errors.

@arcanis
Copy link
Member

arcanis commented Mar 6, 2019

If Yarn is not able to handle"*" versions the way npm does, it should at least warn the user that this syntax is not supported they way the npm ecosystem expects.

I think you're missing the point, which is that this syntax is supported the exact same way as npm in that we make the same guarantees. Or rather that neither of us make the guarantees that @types is relying on. If the case you describe works with npm it's purely thanks to an implementation detail, and while it would make sense to do something similar for the sake of optimizing the tree a bit more relying on a package manager doing so is invalid and broken. Which kinda downgrade the priority of fixing it on my side (anyone is welcome to open a PR, though).

As much as I like TS and the DefinitelyTyped project, it's quite clear that they didn't properly consider the semantic implications behind using regular dependencies. Trust me it's as annoying for me to explain the same problem multiple times (without anyone starting a discussion to fix that on the TS side) as it is for you to experience it ...

@mikl
Copy link

mikl commented Mar 6, 2019

It may be that npm has not explicitly committed themselves to supporting this usage, but as long as it keeps working, I think the odds are fairly slim that a community effort like DefinitelyTyped will bother to rewrite the dependencies of thousands of packages, just to make things work with Yarn.

I understand it's annoying, but considering the positively herculean effort in trying to get the DT community to change, to support a minority package manager like Yarn, I don't think it's particularly surprising that nobody has volunteered yet.

Don't get me wrong, I really like Yarn and I admire all the people who've invested their time in pushing the npm ecosystem forwards by creating and maintaining an alternative to npm. But not so much that I want to spend weeks, if not months, trying to get all of DefinitelyTyped to change.

In fact, given the growing popularity of TS, I think if npm will find themselves having to support this usage. A new version of npm that doesn't work with TypeScript would go over like a lead balloon.

@octogonz
Copy link

octogonz commented Mar 6, 2019

I think the odds are fairly slim that a community effort like DefinitelyTyped will bother to rewrite the dependencies of thousands of packages, just to make things work with Yarn.

I talked to the compiler owners about this a couple years ago, and they eventually convinced me that the * versions were the best possible design, even though it seems strange.

The basic issues are:

  1. The @types version numbers are meant to have the same major/minor as the underlying library, since otherwise it would be extremely confusing to figure out which one you need
  2. This means a SemVer patch-level increment can have breaking API changes (e.g. if the typings strategy was improved, a TypeScript interface was renamed, etc).
  3. Thus @types packages do NOT follow SemVer, and it's a huge mistake to use ranges such as ^1.2.3. The only sensible forms are either locked 1.2.3 or star * (star meaning it's someone else's responsibility to choose the version)
  4. If every @types package were to use locked versions for its dependencies, the entire graph would become locked, meaning someone would need to centrally coordinate huge sets of typings versions that are meant to go together. This is infeasible.
  5. Also, locked dependencies don't make sense for ambient typings (e.g. node or dom apis), which describe a global environment for the application, and are merely the context in which a library runs. The library shouldn't be trying to lock the NodeJs typings version.
  6. Whereas alternatively, if the @types packages always use star dependencies, then it creates a multivariable equation that someone needs to solve, i.e. tinker with the versions to find a set of typings that compile without syntax errors and match the underlying js libraries. The end application project is in the best possible position to solve this equation. Thus * dependencies are a pretty good model, assuming the package manager interprets it to mean the end application decides. (This is technically not what star means, as the Yarn maintainers have pointed out. But there are ways to handle that.)

Part of the problem is that the typings are maintained by volunteers who are typically not part of the release process for the underlying library, and often are unable to deliver a quality bar beyond whatever they need to get their immediate problem unblocked. This is why @types packages encounter a lot of issues that don't seem to come up when library maintainers ship typings as part of the official package.

Despite all these limitations, DefinitelyTyped works amazingly well in practice, at least from the perspective of end users who consume @types packages without being exposed to the sometimes tricky process of how they are produced.

And I agree that NPM/PNPM do work very reliably with star dependencies. Theory aside, Yarn clearly has frequent problems that the other package managers do not seem to encounter in practice.

@arcanis
Copy link
Member

arcanis commented Mar 6, 2019

I talked to the compiler owners about this a couple years ago, and they eventually convinced me that the * versions were the best possible design, even though it seems strange.

I don't have anything against * - what I don't totally understand is the reason why those dependencies aren't peerDependencies rather than regular dependencies. If it is the job of someone else to pick the version they should resolve to, then it seems to me that they clearly match the use case for peer dependencies - since they precisely allow the parent to influence the children.

If @types/react-dom was listing @types/react as a peer dependency, then it would 1/ resolve this problem (because @types/react would by definition have the same resolution as the one provided by the package depending on @types/react-dom), and 2/ make it explicit to the consumer of @types/react-dom that they also need to depend on @types/react.

I understand it's annoying, but considering the positively herculean effort in trying to get the DT community to change, to support a minority package manager like Yarn, I don't think it's particularly surprising that nobody has volunteered yet.

Yep I totally understand - I spend myself a lot of time advocating for good practices and submitting PRs to fix invalid dependency listings as I stumble upon them. While it's not always the most interesting PRs, I feel like it's important to progressively put back some strictness in the ecosystem. Javascript popularity grew fast, and I think we're reaching a pivotal moment where we need to strengthen our foundations (and tools) rather than doubling down whatever the cost.


Anyway, regardless of my personal opinion, I would be willing to review a PR that would help improve this use case and add a regression test to protect against. Any brave soul out there? 🙂

@octogonz
Copy link

octogonz commented Mar 6, 2019

I don't have anything against * - what I don't totally understand is the reason why those dependencies aren't _peer_Dependencies rather than regular dependencies. If it is the job of someone else to pick the version they should resolve to, then it seems to me that they clearly match the use case for peer dependencies - since they precisely allow the parent to influence the children.

In the past (e.g. at least as recently as 4.x), NPM's behavior when installing peer dependencies was so misguided that peer dependencies were effectively useless. (Basically, if you had a dependency chain App->MiddleWare->Library, the MiddleWare package was unable to specify versions for peer dependencies declared by the Library.) During this era, people got frustrated with peer dependencies, and they were widely deprecated. PNPM and Yarn don't have this flaw, and it may be the case that NPM eventually fixed it as well. Recently people seem to be using peer dependencies more, and I haven't heard as many complaints about them.

@arcanis if all of the @types packages declared peer dependencies using * for the version, would Yarn do the right thing?

@zkochan would PNPM handle this okay?

I understand it's annoying, but considering the positively herculean effort in trying to get the DT community to change, to support a minority package manager like Yarn, I don't think it's particularly surprising that nobody has volunteered yet.

I wouldn't personally consider this to require a "herculean effort." If someone could persuasively demonstrate that a different design would significantly improve the DT experience, the code it is pretty much all in one big master branch. It's probably not a big deal to make a sweeping change.

@arcanis
Copy link
Member

arcanis commented Mar 6, 2019

if all of the @types packages declared peer dependencies using * for the version, would Yarn do the right thing?

I think so - the main thing being that it would be the responsibility of the application authors to add those packages to their dependencies (whereas now if you add @types/react-dom you get @types/react "for free" in a completely unintended way). So probably major version bump (or patch since types packages don't follow semver).

Btw, I think I'd be open to make @types packages use exact versions by default when running yarn add (instead of the regular ^), if that can help you (since you mention they never respect semver).

@zkochan
Copy link

zkochan commented Mar 7, 2019

@zkochan would PNPM handle this okay?

I believe the solution described by @arcanis will work with pnpm as well. Pnpm supports peer dependencies and resolves them correctly.

Btw, I think I'd be open to make @types packages use exact versions by default when running yarn add (instead of the regular ^), if that can help you (since you mention they never respect semver)

off topic, I'd do that for everything :-)

@octogonz
Copy link

octogonz commented Mar 7, 2019

Btw, I think I'd be open to make @types packages use exact versions by default when running yarn add (instead of the regular ^), if that can help you (since you mention they never respect semver)

off topic, I'd do that for everything :-)

rush add by default writes ~1.2.3 instead of ^1.2.3, and for @types packages it uses 1.2.3. We've been doing this for a while, and people seem pretty happy with this behavior.

if all of the @types packages declared peer dependencies using * for the version, would Yarn do the right thing?

I think so - the main thing being that it would be the responsibility of the application authors to add those packages to their dependencies

Proposal: Maybe Yarn/PNPM could help out by adding a new command line option --install-types-as-peers. With this option enabled, when the package manager encounters this:

{
  "name": "@types/lib1",
  "version": "1.2.3",
  "dependencies": {
    "@types/lib2": "*"
  }
}

...it would get installed as if it had been written like this:

{
  "name": "@types/lib1",
  "version": "1.2.3",
  "peerDependencies": {
    "@types/lib2": "*"
  }
}

And when the package manager encounters this:

{
  "name": "lib3",
  "version": "1.2.3",
  "dependencies": {
    "@types/lib1": "1.2.3"
  }
}

...it would get installed as if it had been written like this:

{
  "name": "lib3",
  "version": "1.2.3",
  "dependencies": {
    "@types/lib1": "1.2.3"
  },
  "peerDependencies": {
    "@types/lib2": "*"
  }
}

This would allow people to opt-in to this new model and test the waters. If many professional code bases find that this approach works reasonably well, then we could use that evidence to persuade DefinitelyTyped to convert all their packages to adopt it.

@milesj
Copy link

milesj commented Mar 7, 2019

Unless I missed something in this comment chain, everyone keeps talking about the consumer installing all the @types dependencies that are required in their application. But this doesn't seem to take into account all the NPM packages that includes @types in their dependencies, which I would assume is quite a bit, and mostly fixed/caret versions, and not *.

For example, just look at @types/react, which is being depended on more than 2000 times (not all of which are other @types packages). https://www.npmjs.com/package/@types/react

I don't see how peerDependencies is the solution here. The consuming application/library should not have to install @types for literally every NPM package in it's tree that requires them.

@octogonz
Copy link

octogonz commented Mar 7, 2019

But this doesn't seem to take into account all the NPM packages that includes @types in their dependencies, which I would assume is quite a bit, and mostly fixed/caret versions, and not *.

I just did a quick audit of the 2,131 NPM packages consumed by a real production repo. This includes internal packages and external packages from npmjs.com, and a mixture of browser libraries and NodeJS libraries.

Ignoring devDependencies, the dependencies sections contain a total of 147 references to @types packages. Of those, 121 are locked versions and 3 are star versions. Only 23 of the lines are SemVer ranges, and here's the list:

rangedependencyconsuming package
^1.0.7@types/applicationinsights-js@mskeros/chisel
^15.6.1@types/create-react-class@ms/project-web-shared
^1.2.3@types/d3-shape@uifabric/charting
^2.1.0@types/d3-time-format@uifabric/charting
^2.2.29@types/invariantreact-dnd
~1.4.0@types/jju@microsoft/node-core-library
^4.14.116@types/lodash@mskeros/chisel
^4.14.109@types/lodashreact-dnd
^4.14.109@types/lodashreact-dnd-html5-backend
^0.0.32@types/mz@pnpm/link-bins
^0.0.32@types/mz@pnpm/package-bins
^9.6.5 || 10@types/node@pnpm/link-bins
^9.4.0 || 10@types/node@pnpm/logger
^8.0.47@types/nodeadal-node
^10.3.0@types/nodereact-dnd
^15.5.3@types/prop-typesreact-dnd
^0.25.20@types/ramda@pnpm/link-bins
^16.3.14@types/reactreact-dnd
^16.3.16@types/reactreact-dnd-html5-backend
^16.0.5@types/react-domreact-dnd
^1.0.1@types/react-dom-factories@ms/project-web-shared
^0.2.2@types/shallowequalreact-dnd
^0.2.2@types/shallowequalreact-dnd-html5-backend

My guess is that only a few of these packages are actively churning. Most have either (1) not had a SemVer-compatible release in a long time, or else (2) are very well-behaved APIs such as lodash.

Other may have different experiences. What I observed was that during the evolution of this repo, we spent a long time dealing with frustrating breaks caused by patch-increments for DefinitelyTyped packages, until we eventually managed to lock down a consistent set of version numbers for our application and all its dependencies. That's why the set of ^ and ~ ranges is now very small.

To summarize:

  • SemVer ranges cause annoying breaks for @types packages
  • If every library locks its versions in dependencies, then you get massive fragmentation of side-by-side installs, including problems such as the compiler getting confused by two different typings for the same API
  • The reliable solution is for each library to lock its own tree of versions using devDependencies. This allows consumers of a library to arrive at their own solution.
  • (You can get away with other approaches for certain well-behaved edge cases such as above, but those approaches break down when propagated across a large enough dependency graph.)
  • If you're looking for an "ideal" solution, make a PR to add the .d.ts files to the upstream JavaScript package, and get the owners to commit to supporting TypeScript. @types is merely a hacky step towards the right thing.

@milesj
Copy link

milesj commented Mar 7, 2019

Ideally, if all of these libraries used * versions for @types in dependencies, would that alleviate most of these install issues? That way, latest is always used when installing transitive, and applications can lock their @types versions as they will.

Could this also be solved by a Yarn v2 plugin?

@arcanis
Copy link
Member

arcanis commented Mar 7, 2019

Note that the problem isn't about resolving to latest. It's just that, at a basic level, Yarn will only ever resolve one range to one version, regardless of the position of the package in the dependency tree (this is different from npm which might resolve the same range to different versions depending on the package location on the dependency tree, for the better or worse). This mechanism is a cornerstone of our architecture, and unlocks various core optimizations.

From an implementation perspective, imagine that there is a Map whose key is a range and the value is the resolution. When we see a package depending on something, we check whether the range has been resolved and, if it hasn't, we enqueue it to be resolved later. Since the ranges resolve to the same versions regardless of the package that required them, we don't really factor the tree location when considering which version should be used for a specific range1 - hence why ^15.0.0 and * get different result2.

Could this also be solved by a Yarn v2 plugin?

Technically you could have a resolver that would bind itself on @types/* resolutions and would convert the dependencies into peerDependencies on the flight, but that definitely wouldn't be something I'd recommend. Plugins should extend the spec, not replace it - especially when more correct solutions have been identified.


1 This isn't entirely true in that we do try to apply some optimizations when possible - but generally speaking you should assume that we can't

2 If you were to suggest that we simply reuse the result from ^15.0.0 when resolving *, consider that the resolution order matters: what if * is resolved before ^15.0.0? Fixing this would be quite hard, would potentially cause deoptimizations of the resolution process, and ultimately would be a lot of effort to circumvent a problem that originates in the first place in how packages are defined.

@gitowiec
Copy link

gitowiec commented Apr 29, 2019

Hi, I am looking for a practical way to cope with this issue in monorepo. Could You help?
I added such lines to .yarnrc

save-prefix ""
--install.check-files true
--add.check-files true
--remove.check-files true
--install.frozen-lockfile true
--add.frozen-lockfile true
--remove.frozen-lockfile true

It helped me when the @types/react dependency is hoisted, but when Yarn decides to keep it nested inside specific dependency package's node_modules (eg @types/redux-form) I still get multiple TypeScript errors during compilation similar to this:

TS2717: Subsequent property declarations must have the same type.  Property 'a' must be of type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>', but here has type 'DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>'.

How to resolve this issue in monorepo? I will add that Yarn's "resloutions" don't work in monorepos :(

(Using Yarn 1.15.2)

@mikl
Copy link

mikl commented Apr 29, 2019

@gitowiec I think the only solution atm. is to switch to npm with Lerna. Maybe Tink in the future.

@gitowiec
Copy link

@mikl Thanks for the reply. But your answer makes me sad. We have a small devs team and this issue is pushing us back. Until now I "fixed" this problem by manually editing yarn.lock. It helped when packages where hoisted from packages/ui workspace to the workspaces root.

@octogonz
Copy link

octogonz commented Apr 29, 2019

We've been using PNPM 3.x which introduced a --resolution-strategy=fewer-dependencies option that so far seems to solve this problem.

@rjmunro
Copy link
Contributor

rjmunro commented May 16, 2019

From my reading of what dependencies, devDependencies and peerDependencies are supposed to be (Ignoring bugs in yarn, npm etc.) @types/... packages should:

  • Always be in devDependencies for normal projects or libraries as they are only needed for compilation, not for runtime.
  • Always be in peerDependencies for @types/ that depend on each other, because it is never valid to have different versions installed at the same time. Currently the typescript compiler cannot cope with such a setup, and it probably wouldn't make sense if it did.
  • Consider peer-depending on the thing whose types they are defining, if installing from npm is the only sane way to get that thing.

@octogonz
Copy link

* Always be in peerDependencies for `@types/` that depend on each other,  because it is never valid to have different versions installed at the same time. 

Well, it is never valid for ambient declarations. For normal declarations it is valid in cases where different versions of the underlying library were installed (and are not being used interchangeably).

@eps1lon
Copy link
Member

eps1lon commented May 16, 2019

Always be in peerDependencies for @types/ that depend on each other, because it is never valid to have different versions installed at the same time. Currently the typescript compiler cannot cope with such a setup, and it probably wouldn't make sense if it did.

Declarations published under @types/ always declare their dependencies as actual dependencies not as peerDependencies. I would agree that it would be better to have those as peers but as long as the biggest types repo is using direct dependencies we should make it work for those.

@milesj
Copy link

milesj commented May 16, 2019

@rjmunro

Always be in devDependencies for normal projects or libraries as they are only needed for compilation, not for runtime.

This just isn't true. If a library uses a type from a @types package in their public API (argument type, return type, etc), then the @types package must be a normal dependency, otherwise the TS compiler will fail.

Always be in peerDependencies for @types/ that depend on each other, because it is never valid to have different versions installed at the same time. Currently the typescript compiler cannot cope with such a setup, and it probably wouldn't make sense if it did.

What about @types packages that have different types between major versions? Using * as a peer could theoretically pull in a much higher version than the app/library in question. But this is definitely something tsc should resolve.

@milesj
Copy link

milesj commented Mar 28, 2020

I started getting around this by running this alias every time I messed with deps.

alias yeet='npx yarn-deduplicate yarn.lock && yarn install'

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

No branches or pull requests

9 participants