Selective versions resolutions #68

victornoel · 2017-05-21T09:47:39Z

Hi,

Here is an RFC for solving the situation discussed in yarnpkg/yarn#2763

xtuc · 2017-05-21T09:54:42Z

accepted/0000-selective-versions-resolutions.md

+the `package.json` file.
+
+ça veut dire qu'on doit pouvoir mettre des specs invalides (wrt vesion) dans
+lockfile.


Why is this written in french? Should we translate this part?

haha no, sorry, I took some notes while writing and forgot to remove them :P I'm going to amend the commit!

It's done, thanks :)

sejoker · 2017-05-21T16:38:05Z

accepted/0000-selective-versions-resolutions.md

+When a nested dependency is being resolved by yarn, if the `resolutions` field
+contains a version for this package, then this version is used instead.
+
+All the examples are given with exact dependencies, but note that putting a


it make sense to provide separate examples for the resolutions with non-exact specification. The more specific RFC will be defined the better end implementation will be produced at the end.

Ok, I will work on that soon then :)

My idea is that the way yarn resolves a given dependency is the same as before, so exact or non-exact specifications should be handled the same.
For example, the role of yarn.lock doesn't change, it is completely inferred from the package.json file, including the resolutions field.

But maybe there is some corner cases to be discovered. Im' thinking for example about a situation where a nested dependency has a specification more specific than what is specified in the resolutions field. Intuitively I would say that the resolutions field takes precedence and a warning is printed. I will put that in a more formal way in the document :)

it's done :)

sejoker · 2017-05-21T16:42:24Z

accepted/0000-selective-versions-resolutions.md

+But the `resolutions` field is always considered by yarn, even if `--flat` is
+not specified.
+
+Inceidently, this resolves this strange situation when two developers would be


Incidentally

sejoker · 2017-05-21T16:54:55Z

@victornoel in yarnpkg/yarn#2763 you mentioned that npm allows overriding dependencies. Could you reference that in RFC? I can't find any documentation on npm website, maybe I missed something. The point is, referencing npm's similar mechanism will increase chances of the RFC to land.

victornoel · 2017-05-21T17:08:08Z

@sejoker actually I'm not so sure it is possible with npm, I will look a bit more, but what I originally linked (https://nodejs.org/en/blog/npm/managing-node-js-dependencies-with-shrinkwrap/) is actually closer to the --flat options than to this RFC.

I still remember reading something about that but there is a lot of chance I was wrong.

EDIT: actually it is closer to the lockfile mechanism than to --flat apparently, so it's out of scope I think :)

bestander · 2017-05-22T17:16:42Z

accepted/0000-selective-versions-resolutions.md

+
+Should yarn warn the user about an incoherence between an explicit dependency
+and a resolution. For example if the user specify a dependency to
+`typescript@2.3.2` and the resolutions field contains `typescript@2.3.0`.


Considering that Yarn warns when peer dependencies don't match semver I think overridden resolutions should warn as well.
I think this is a temporary measure until the community catches up and fixes issues

bestander · 2017-05-22T17:17:40Z

accepted/0000-selective-versions-resolutions.md

+For sure if the above alternative solution is chosen, this wouldn't make sense.
+
+Should we warn if a resolutions is incompatible, but still upper-bounded?
+For example, forcing version `a@2.3` while a dependency needs version `a@2.2`


I think if semver is violated there should be a warning.

bestander · 2017-05-22T17:20:21Z

accepted/0000-selective-versions-resolutions.md

+    "typescript": "2.3.2"
+  },
+  "resolutions": {
+    "@angular/cli": {


I would prefer nesting to be an option.
JS project, unlike Java, have hundreds of dependencies, it is not rare to have several versions of the same library at different levels and this is fine.
Sometimes I'd need a fine control to override resolution only for a specific dependency path

yes, that's exactly why I am not so sure the original proposition is good enough and I added that question at the end :)

Should we merge both of the proposition?

Accepting both resolutions like this:

"typescript": "2.3.2"

and

"@angular/cli": { "typescript": "2.3.2" }

bestander · 2017-05-22T17:21:45Z

You need to consider check --verify-tree and check commands.
First one parses package.json files and verifies that they don't conflict with each other.

bestander · 2017-05-22T17:22:00Z

Thanks for raising this, @victornoel!

arcanis · 2017-05-22T17:26:02Z

Three thoughts:

Maybe we could put this inside the lockfile instead of the package.json? I assume this resolutions key will have no effect unless it's inside the project root, right (just like we ignore nested yarn.lock)?
Maybe we could make the resolutions key a glob pattern, minus the node_modules:
```
{ "resolutions": { "**/left-pad": "1.0.0", "**/@angular/cli/typescript": "ssh+git://..." } }
```
It would be more verbose, but this feature should be a very advanced feature anyway, and should never be used under normal circonstances, so I think it could work.
Maybe the resolutions could become a "preference" array, so that :
```
{ "resolutions": { "typescript": [ "ssh+git://..." ] } }
```
Then, when resolving, we would first try the dependencies listed in this array, in the order defined. If they don't match the version specified, then continue just like we currently do. The problem would be finding a good wording for the exotic resolvers (ie dependencies that don't come from the npm registry).

bestander · 2017-05-22T17:38:55Z

Maybe we could put this inside the lockfile instead of the package.json?

I think this list will be manually managed and yarn.lock file is automatically generated.
Also IMHO resolutions is very similar dependencies from package.json, they loosely influence resolution step in Yarn install command, yarn.lock definitely describes resolved dependencies after resolution happened.

victornoel · 2017-05-23T06:54:15Z

@arcanis originally it is not intended to be a very advanced feature. It is meant to be used regularly by people. At least in the Maven world, this kind of feature is very useful and thus very used.

Your question raises a point I didn't think of before: is it the case that you can have node_modules directory inside dependencies that are themselves in the node_modules of one of the project dependency?
Something like myproject/node_modules/dep1/node_modules/dep2/node_modules/dep3?

bestander · 2017-05-23T09:53:26Z

@victornoel, feel free to start work on the implementation, we can agree on the details how specifiers are implemented in the meantime.

bestander · 2017-05-23T09:54:09Z

Something like myproject/node_modules/dep1/node_modules/dep2/node_modules/dep3

That is a legit use case.

victornoel · 2017-05-23T10:01:02Z

feel free to start work on the implementation, we can agree on the details how specifiers are implemented in the meantime.

@bestander I'm not sure I have the skills to implement this, I wouldn't even know where to start…
I can commit to take some time to see how it could be implemented and see if I'm up to the task, but I would need you to give me some guidance on where to start for this to happen.

Something like myproject/node_modules/dep1/node_modules/dep2/node_modules/dep3

That is a legit use case.

It is? Damn, it means that to be complete, we would need to be able to specify resolutions like this then:

"resolutions": {
    "dep1": {
        "dep2": {
            "dep3": "1.0.0"
        }
    }
}

Or use some kind of glob pattern as proposed by @arcanis

bestander · 2017-05-23T10:20:49Z

I'm not sure I have the skills to implement this, I wouldn't even know where to start…

Most of resolution stuff is happening here https://github.com/yarnpkg/yarn/blob/master/src/package-resolver.js.
You can also search where semver module is used because your change is going to be about violating semver.

Or use some kind of glob pattern as proposed by @arcanis

For deep trees, which is common, a glob pattern makes sense.
A maybe easier alternative is to override a specific semver with another one, e.g. typescript@>=2.5 < 3 => typescript@4.0.2

victornoel · 2017-05-23T11:45:23Z

A maybe easier alternative is to override a specific semver with another one, e.g. typescript@>=2.5 < 3 => typescript@4.0.2

I'm not very convinced by this approach, I explained why in the RFC (I will update it with all the feedbacks and discussions soon by the way).

bestander · 2017-05-23T11:47:25Z

It just seemed easier to implement for me but I don't insist

…

On 23 May 2017 at 12:45, Victor Noël ***@***.***> wrote: A maybe easier alternative is to override a specific semver with another one, e.g. typescript@>=2.5 < 3 => ***@***.*** I'm not very convinced by this approach, I explained why in the RFC (I will update it with all the feedbacks and discussions soon by the way). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACBdWJik43CvTwRP_haOcjfLNr0PIBUjks5r8sbVgaJpZM4NhjLr> .

victornoel · 2017-05-23T11:55:39Z

It just seemed easier to implement for me but I don't insist

I agree it will be easier to implement, I'm more worried about usability and expressiveness :)

bestander · 2017-05-25T10:15:22Z

accepted/0000-selective-versions-resolutions.md

+    "@angular/cli": "1.0.3"
+  },
+  "resolutions": {
+    "typescript": "2.3.2"


I still think it is a bit too brute force to force typescript to 2.3.2 everywhere.
We need to be able to enforce typescript to some subtree.
What do you think of the proposed glob for the resolution path?

The use case I am trying to solve is this:

left-pad@2.0.5 is getting released with a bug

it is used all over the app dependencies, some of them get locked on @1.0.1 which is fine

you want to upgrade jest to next version but some of the subdependencies requires "leftpad@^2.0.0" which will get resolved to "leftpad@2.0.5"

you want to force jest's subdependency to leftpad@2.0.4 but don't change force it across the whole app

I would think this could solve it elegantly.

"resolutions": { "jest-*/**/left-pad": "1.0.4" }

What do you think?

yes, I agree, I was actually thinking of something like that after integrating in my head all the comments and @arcanis glob pattern proposition :)

I'm still processing all that was said and also things to add (check command in particular) to produce an update of the RFC.

bestander · 2017-05-25T10:15:57Z

@victornoel, this is one of the best RFC I've seen here, great job!

victornoel · 2017-05-25T10:21:26Z

hehe, cool, thanks!

bestander · 2017-06-29T07:48:37Z

accepted/0000-selective-versions-resolutions.md

+
+## Package designation and single `*`
+
+I am not totally convinced that using a single `*` in a package designation


Ok, let's not do single *

bestander · 2017-06-29T07:50:29Z

accepted/0000-selective-versions-resolutions.md

+I wonder if the `--flat` option of `install` shouldn't be transformed to
+a `flatten` command that would:
+1. Fill in the *resolutions* for all nested dependencies.
+2. Set the `flat` field in the `package.json`.


I like that idea.
Unlike --production, --flat is an option that you decide for the project early on and stick to it.

bestander · 2017-06-29T07:53:18Z

accepted/0000-selective-versions-resolutions.md

+
+## resolutions in dependencies
+
+If a dependency contains resolutions, should it be taken into account?


No, same as shrikwraps/lockfiles, every project is responsible for their resolutions.
Also right now this is a Yarn-only feature, we don't really want to get it into the wild yet too much

bestander · 2017-06-29T07:58:22Z

accepted/0000-selective-versions-resolutions.md

+2. `install` should be only about building the `node_modules` directory, not
+modifying the the `package.json` IMHO.
+
+If we do that, then the `--flat` option of `add` (and the `flat` option in the


I don't understand how much --flat affects resolutions feature.
Can we decide on flat in a follow up RFC?

If --flat gives trouble to this feature then let's just throw on --flat if resolutions property is specified

I'm not sure I understand what you mean:

Currently, --flat is both about populating resolutions field AND taking resolutions field into account when doing install (including install after add).

This RFC is about taking resolutions field into account when doing install (including install after add).

So with this RFC, --flat is only about populating resolutions field.

Thus the question is when should the resolutions field be populated.
Maybe this can simply be managed in another RFC that transforms --flat into:

a flatten command that flattens the current dependency tree.

a --flat option (for all package.json modifying command such as add) as well flat field to keep the dependency tree flattened at all time.

I agree, it seems like I might not have all the context on this one.
I thought that --flat was just enforcing that every dependency is resolved only once so that after hoisting we get a flat structure in node_modules.

I propose leaving --flat behavior out of scope for this RFC and move forward with the implementation if it is possible.
We can follow up with --flat specific right after it.

arcanis · 2017-06-30T16:44:53Z

accepted/0000-selective-versions-resolutions.md

+```
+
+yarn will use `package-d1@3.0.0` both for `package-a` and the nested
+dependency `package-a` of `package-c`.


Why? **/package-a/package-d1 doesn't match package-a

Unless you mean that both /package-a/package-d1 and /package-c/package-a/package-d1 will be fixed at 3.0.0?

Unless you mean that both /package-a/package-d1 and /package-c/package-a/package-d1 will be fixed at 3.0.0?

yes: /package-a/package-d1 and /package-c/package-a/package-d1 both match **/package-a/package-d1

arcanis · 2017-06-30T16:49:24Z

accepted/0000-selective-versions-resolutions.md

+With the current proposal, the package designation `package-a` is an alias for
+`**/package-a`: this means the behaviour of yarn with a project whose
+`resolutions` field contains *resolutions* filed by a pre-RFC yarn will be
+as expected: the nested dependencies will have the fixed version specified.


Good idea. I also think we should prevent people from using both pkg-name and glob syntaxes at the same time. If they want the same effect, they should explicitely use **/pkg-name.

Do you mean:

consider both syntaxes equally valid but prevent the user from mixing them in the same project, or

have a preference for the glob syntax and keep the non-glob syntax only for retro-compatibility purposes?

How would we achieve one or the other btw? Via printing a warning? For 2. I can easily see it happening, but for 1. it would seem strange I think...

arcanis · 2017-06-30T16:56:54Z

This RFC seems to be in a really good shape @victornoel, kudos! 😃 What would you think about implementing a first iteration in Yarn and see how it goes? Would you be willing to do it?

victornoel · 2017-07-02T16:16:57Z

@arcanis last time I checked the code for this task, I got a bit lost in it, I'm worried I won't succeed doing it and I wouldn't want to commit to something I can't do :)
@bestander seemed to know which specific places would be concerned (see #68 (comment)), so, once we consider the set of features to implement fixed, I can look at the code again with @bestander guidance on where to look (I suppose this won't be different from what he envisioned in the above comment) and tell you if I can do it or not :)

I'm going to introduce the changes discussed in last few days very soon and then I will explore the code again!

bestander · 2017-07-03T17:15:23Z

@victornoel, you'll have our full support.
Feel free to push some half-baked ideas in and we can have a look.
Maybe you could start with a PR that adds failing and skipped tests that cover the spec?
Then with another go we would just need to comply with the tests

bestander · 2017-07-03T17:16:59Z

Personally, I think this is one of the most important features in while for Yarn.
It addresses a huge point of stress for semver packages users.

victornoel · 2017-07-03T17:27:14Z

Maybe you could start with a PR that adds failing and skipped tests that cover the spec?

Actually I've already started doing that some time ago :) it's a good idea, and like this, worst case scenario is someone will be able to implement the RFC from the failing tests :)

victornoel · 2017-07-08T09:10:07Z

ok, I pushed the last changes, I kept the discussion on the future of --flat in a specific section at the end for record, and later we should make another RFC.

bestander · 2017-07-10T07:13:38Z

Great job, @victornoel.
This is going to be huge!

HaleTom · 2017-10-06T10:58:09Z

Where do I find the official location of this RFC (and its number)?

bestander · 2017-10-17T21:00:12Z

@HaleTom, it lives in this repo after it is merged https://github.com/yarnpkg/rfcs/blob/master/accepted/0000-selective-versions-resolutions.md

victornoel · 2017-10-18T07:23:11Z

@bestander by the way, maybe it should be moved to the implemented sub-folder now, no?

bestander · 2017-10-18T18:16:12Z

Of course, do you want to send a quick PR?

victornoel · 2017-10-19T10:40:13Z

ok, I will do that in the next few days! :)

victornoel mentioned this pull request May 21, 2017

Handling dependencies of dependencies and their resolution yarnpkg/yarn#2763

Closed

xtuc reviewed May 21, 2017

View reviewed changes

Selective versions resolutions

98693bd

victornoel force-pushed the selective-versions-resolutions branch from 5533347 to 98693bd Compare May 21, 2017 09:56

sejoker reviewed May 21, 2017

View reviewed changes

orthograph and max line width

a860d64

Added a section on non-exact specifications

3eccabe

bestander reviewed May 22, 2017

View reviewed changes

bestander mentioned this pull request May 23, 2017

yarn installs unpublished npm packages yarnpkg/yarn#3274

Closed

clarified the questions about warning

04dd7cc

bestander reviewed May 25, 2017

View reviewed changes

bestander reviewed Jun 29, 2017

View reviewed changes

arcanis mentioned this pull request Jun 30, 2017

Regression: Pinning dependencies behaves differently than npm. yarnpkg/yarn#3775

Closed

arcanis reviewed Jun 30, 2017

View reviewed changes

bestander mentioned this pull request Jun 30, 2017

yarn --flat sees a version conflict where there is none yarnpkg/yarn#3780

Closed

final changes, no more open questions

bc3e518

victornoel mentioned this pull request Jul 9, 2017

Selective versions resolutions yarnpkg/yarn#3868

Closed

3 tasks

bestander merged commit fd710e7 into yarnpkg:master Jul 10, 2017

jeanduplessis mentioned this pull request Jul 25, 2017

Vulnerability in crypto-browserify aws/aws-sdk-js#1602

Closed

kaylie-alexa mentioned this pull request Aug 6, 2017

Selective version resolution feature yarnpkg/yarn#4105

Merged

BYK mentioned this pull request Aug 22, 2017

Docs: document selective version resolutions yarnpkg/website#605

Closed

victornoel deleted the selective-versions-resolutions branch August 24, 2017 07:23

cif mentioned this pull request Oct 13, 2017

[TRACKING] Update to use inline-style-prefixer v3 FormidableLabs/radium#918

Closed

bestander mentioned this pull request Oct 24, 2017

Workspaces - Is it possible to exclude some node_modules from beeing hoisted to the root node_modules folder? yarnpkg/yarn#3882

Open

farcaller mentioned this pull request Dec 31, 2017

Run typescript sources through full babel transpiling pipeline gatsbyjs/gatsby#3369

Merged

4 tasks

jglover mentioned this pull request Jan 15, 2018

Pinning react version 15.6.2 auth0/lock#1142

Merged


		## Package designation and single `*`

		I am not totally convinced that using a single `*` in a package designation


		## resolutions in dependencies

		If a dependency contains resolutions, should it be taken into account?

Selective versions resolutions #68

Selective versions resolutions #68

Conversation

victornoel commented May 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sejoker May 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sejoker commented May 21, 2017 • edited Loading

victornoel commented May 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victornoel May 23, 2017 • edited Loading

Choose a reason for hiding this comment

bestander commented May 22, 2017

bestander commented May 22, 2017

arcanis commented May 22, 2017 • edited Loading

bestander commented May 22, 2017

victornoel commented May 23, 2017

bestander commented May 23, 2017

bestander commented May 23, 2017

victornoel commented May 23, 2017

bestander commented May 23, 2017

victornoel commented May 23, 2017

bestander commented May 23, 2017 via email

victornoel commented May 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bestander commented May 25, 2017

victornoel commented May 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arcanis commented Jun 30, 2017

victornoel commented Jul 2, 2017

bestander commented Jul 3, 2017

bestander commented Jul 3, 2017 • edited Loading

victornoel commented Jul 3, 2017 • edited Loading

victornoel commented Jul 8, 2017

bestander commented Jul 10, 2017

HaleTom commented Oct 6, 2017

bestander commented Oct 17, 2017

victornoel commented Oct 18, 2017

bestander commented Oct 18, 2017

victornoel commented Oct 19, 2017

sejoker May 21, 2017 •

edited

Loading

sejoker commented May 21, 2017 •

edited

Loading

victornoel commented May 21, 2017 •

edited

Loading

victornoel May 23, 2017 •

edited

Loading

arcanis commented May 22, 2017 •

edited

Loading

bestander commented Jul 3, 2017 •

edited

Loading

victornoel commented Jul 3, 2017 •

edited

Loading