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

Surprises around resolutions #2202

Open
borekb opened this issue Dec 3, 2020 · 19 comments
Open

Surprises around resolutions #2202

borekb opened this issue Dec 3, 2020 · 19 comments
Labels
bug Something isn't working upholded Real issues without formal reproduction

Comments

@borekb
Copy link
Contributor

borekb commented Dec 3, 2020

I encountered a few "surprises" around resolutions – not sure if those are expected but since I couldn't find them mentioned on GitHub yet, here they go.

Context

We're using graphql@14 in our project but one of the deep dependencies has a peerDependency on graphql@15, which results in a warning like this:

➤ YN0060: │ @company/my-package@workspace:my-package provides graphql (pc69a0) with version 14.7.0, which doesn't satisfy what @graphql-codegen/typescript-operations and some of its descendants request

The actual package that has this peerDependency is relay-compiler@10.0.1. Since we cannot upgrade graphql in our project, the "solution" (though not great) is to downgrade relay-compiler to 9.x which has peerDependency on graphql@14.

--save / -s doesn't work

I ran this command:

yarn set resolution --save relay-compiler@npm:10.0.1 9.1.0

This updated yarn.lock like this (will be important later):

"@graphql-tools/relay-operation-optimizer@npm:^6":
  version: 6.2.4
  resolution: "@graphql-tools/relay-operation-optimizer@npm:6.2.4"
  dependencies:
    relay-compiler: 10.0.1

"relay-compiler@npm:10.0.1":
  version: 9.1.0
  resolution: "relay-compiler@npm:9.1.0"
  dependencies: ...

But it didn't affect project-level package.json – I expected to find resolutions field there. According to this Discord reply, -s isn't implemented yet. Fair enough but I couldn't find the info here on GitHub so just want to mention it.

resolutions field in package.json doesn't update yarn.lock?

Since I wanted to have a mention of a version mapping in package.json and not just in yarn.lock, I updated the manifest manually:

{
  "name": "@company/project-root",
  "resolutions": {
    "relay-compiler": "9.1.0"
  }
}

I then reverted yarn.lock (I got rid of the changes done by yarn set resolution) and ran yarn (install).

At this point, I expected the yarn.lock to look exactly like above but this was the result instead:

"@graphql-tools/relay-operation-optimizer@npm:^6":
  version: 6.2.4
  resolution: "@graphql-tools/relay-operation-optimizer@npm:6.2.4"
  dependencies:
    relay-compiler: 10.0.1

"relay-compiler@npm:9.1.0":
  version: 9.1.0
  resolution: "relay-compiler@npm:9.1.0"
  dependencies: ...

The key point is that @graphql-tools/relay-operation-optimizer depends on relay-compiler@10.0.1 but this version 10.0.1 is nowhere to be found in the lockfile – the only key there is "relay-compiler@npm:9.1.0".

I guess in this case, Yarn consults both yarn.lock and package.json#resolutions to find out how to resolve relay-compiler but it feels strange to me – I'd expect the mapping to be obvious just from the lockfile itself.


Again, not sure if those are issues or expected behavior but they surprised me.

@borekb borekb added the bug Something isn't working label Dec 3, 2020
@arcanis
Copy link
Member

arcanis commented Dec 3, 2020

resolutions field in package.json doesn't update yarn.lock?

This is expected - the resolutions field only has effect while it's there. It's important, otherwise your ranges would be "eternally" locked to likely incompatible range resolutions.

--save / -s doesn't work

Indeed, we should remove this flag from the documentation for the time being (or implement it) 🤔

@borekb
Copy link
Contributor Author

borekb commented Dec 3, 2020

@arcanis thanks for the explanation. I get what you're saying but it still feels strange that resolutions vs. yarn set resolution lead to two different end states of our source files (although the actual installation will be the same, at least I hope 😄).

Theoretically, could yarn set resolution update only the package.json manifest instead of writing to yarn.lock? That way, both ways would be equivalent.

@arcanis
Copy link
Member

arcanis commented Dec 3, 2020

Theoretically, could yarn set resolution update only the package.json manifest instead of writing to yarn.lock? That way, both ways would be equivalent.

No; the resolutions field and yarn set resolution are two different things. The first is a core-supported feature in itself, the second is meant to do exactly what it does: assign a new resolution to the lockfile. They share the same name because resolution is unfortunately a bit too generic, but they have different purposes.

@arcanis arcanis closed this as completed Dec 3, 2020
@arcanis
Copy link
Member

arcanis commented Dec 3, 2020

Reopening for --save

@arcanis arcanis reopened this Dec 3, 2020
@borekb
Copy link
Contributor Author

borekb commented Dec 3, 2020

Oh, so the two are two different things? Even though the --save is currently not implemented, its docs say this:

If you wish to make the enforced resolution persist whatever happens, add the -s,--save flag which will also edit the resolutions field from your top-level manifest.

This makes intuitive sense to me but it's possible that this docs suggest something that couldn't work / is wrong.

@yarnbot

This comment has been minimized.

@yarnbot yarnbot added the stale Issues that didn't get attention label Jan 2, 2021
@yarnbot yarnbot closed this as completed Jan 7, 2021
@paul-soporan paul-soporan reopened this Jan 7, 2021
@paul-soporan paul-soporan added upholded Real issues without formal reproduction and removed stale Issues that didn't get attention labels Jan 7, 2021
@noahnu
Copy link
Contributor

noahnu commented Mar 3, 2021

What does a "--save" look like? I was playing around with adding save functionality, and the main issue is that if you're setting a resolution for a transitive dependency, you need to specify the full path as the resolution key in the package.json. We can't fallback to globs because that's been removed in yarn v2 (unless it were to be added back?). Is the idea to only support saving for direct dependencies, or do we need to somehow generate the patterns?

I took a stab and trying to generate the patterns, however I don't think there's an existing function which given a descriptor, lists all packages that reference that descriptor? Also ran into an issue comparing descriptors because the npm: protocol is optional, but is not inferred when generating descriptor hashes (ended up re-generating the hashes).

@cianx
Copy link

cianx commented Aug 31, 2021

It appears the -s and --save are still not implemented in 3.0.1. Resolutions only specified in the lock file are not very user friendly as they cannot be easily observed by a developer. The lock file is expected to be a result of configuration not a means of overriding it.

@KyloJorgensen
Copy link

It might be helpful if this is resolved aka faker.js and color.js

@MrChrisRodriguez
Copy link

The documentation is still incorrect/confusing. The -s / --save flag is still being shown even though it doesn't do anything which leads devs down a rabbit whole looking for a bug instead of just knowing there's an absent feature.

@0x80
Copy link

0x80 commented Jul 9, 2022

I'm trying to use resolutions inside of a yarn workspaces repo, because it seems I have a problem with react 18 types clashing with 17 when attempting to use React 17 in a Nextjs 12 app, so this is what I've tried:

yarn set resolution @types/react@npm:18.0.9 17.0.49

I've tried this in the root of the repo and inside the app workspace package, but I see no changes made to the lock file or any other file. Am I missing something?

@steverep
Copy link

Found this issue after being very confused why -s seemed to do nothing...

No; the resolutions field and yarn set resolution are two different things. The first is a core-supported feature in itself, the second is meant to do exactly what it does: assign a new resolution to the lockfile. They share the same name because resolution is unfortunately a bit too generic, but they have different purposes.

I'm also confused by this explanation. If they have different purposes, could you give an example of why you'd prefer to use one method over the other?

If yarn set resolution is intended to produce a different result (and not add to the "resolutions" field), then that just leads to more questions like:

  1. How do you know it was set by a developer? Nothing in the lock file change suggests as much.
  2. How do you undo it other than edit the lock file manually?
  3. Why is yarn patch so different (because its --save works as expected)?

@Blasz
Copy link

Blasz commented Mar 9, 2023

resolutions dependencies not updating yarn.lock is making it harder for me to parse yarn.lock as I now need to also cross-reference the resolutions field to find missing ranges in yarn.lock.

This is expected - the resolutions field only has effect while it's there. It's important, otherwise your ranges would be "eternally" locked to likely incompatible range resolutions.

Couldn't this be solved by not allowing incompatible range resolutions unless explicitly specified in resolutions, similar to what yarn 1 does? I'm assuming allowing incompatible ranges anywhere without resolutions is an intentional decision, what is the reasoning behind it?

Alternatively could something like a resolution: protocol be used to signal that the range is there because of a resolution? When processing the lockfile yarn could then cross-reference that with the resolutions field to see if it is still valid. I'm guessing this wouldn't work since the protocol couldn't actually be used in the resolution field of yarn.lock

@gertsonderby
Copy link

gertsonderby commented Mar 31, 2023

As far as I can tell, the only thing that has changed since this issue was first posted in 2020, is that resolutions in package.json just plain do nothing now, instead of maybe working a bit weirdly like the OP describes. (--save still remains in the documentation, also.)

My own results using yarn@3.2.0:

Excerpt from my package.json:

  "resolutions": {
    "@types/react": "17.0.43",
    "uglify-js@npm:~1.2": "2.6.0",
    "underscore@npm:~1.4.4": "1.12.1"
  },

Output of yarn why uglify-js, note the middle result:

├─ handlebars@npm:4.7.7
│  └─ uglify-js@npm:3.17.4 (via npm:^3.1.4)
│
├─ juicer@npm:0.6.15
│  └─ uglify-js@npm:1.2.6 (via npm:~1.2)
│
└─ pug-filters@npm:3.1.1
   └─ uglify-js@npm:2.8.29 (via npm:^2.6.1)

Excerpt from my yarn.lock as created from scratch from the above package.json:

"uglify-js@npm:^2.6.1":
  version: 2.8.29
  resolution: "uglify-js@npm:2.8.29"
  dependencies:
    source-map: ~0.5.1
    uglify-to-browserify: ~1.0.0
    yargs: ~3.10.0
  dependenciesMeta:
    uglify-to-browserify:
      optional: true
  bin:
    uglifyjs: bin/uglifyjs
  checksum: 24f2ae09b96bbb56cc3802f575ee2cdbc6822d942c6877ee4a5637e949f269e48f4baa8d193c47324cdfc1cc8e6853e1479d26e228be2412bc0da3649eaedb35
  languageName: node
  linkType: hard

"uglify-js@npm:^3.1.4":
  version: 3.17.4
  resolution: "uglify-js@npm:3.17.4"
  bin:
    uglifyjs: bin/uglifyjs
  checksum: 7b3897df38b6fc7d7d9f4dcd658599d81aa2b1fb0d074829dd4e5290f7318dbca1f4af2f45acb833b95b1fe0ed4698662ab61b87e94328eb4c0a0d3435baf924
  languageName: node
  linkType: hard

"uglify-js@npm:~1.2":
  version: 1.2.6
  resolution: "uglify-js@npm:1.2.6"
  bin:
    uglifyjs: ./bin/uglifyjs
  checksum: dbabf3ad42a226c1b70e799331f084f8b2ce7b013266565a4ea0b7cf461c09912dbbae07ac292480db4debc2777e14ab2c4e50eed8bba5e94efd1195052ef38d
  languageName: node
  linkType: hard

As you can see, yarn just went and installed the old version of uglify-js despite any protestations of package.json. No sign of the package it was supposed to redirect to (v2.6.0). Does juicer use a newer version of it? I don't know -- does anyone know a way to check that? Dependabot certainly seems to think not.

@sushruth
Copy link

sushruth commented Apr 5, 2023

@gertsonderby I have had this issue before with yarn. My workaround was to not use semver ranges but use exact versions as reported by yarn why and it would fix that. But that's not a good solution. We need yarn to treat semver ranges in resolutions properly.

@jaybuidl
Copy link

But why not mentioning that -s is not implemented on the docs?
https://yarnpkg.com/cli/set/resolution

@PaulRBerg
Copy link

Ok so how can resolutions be enforced in Yarn v3? It's not clear from this conversation.

Or can they not be enforced at all?

@gertsonderby

This comment was marked as outdated.

@arcanis
Copy link
Member

arcanis commented Jun 6, 2023

You cannot get yarn to follow resolutions set in package.json to my knowledge -- it simply ignores them.

That's incorrect. I mean, we even use them on this very repository.

Since this issue is intended to be specifically about the -s flag (which isn't implemented right now, so you must set the resolutions field yourself as per the documentation syntax), I'll lock it until it's completed. If you find yourself in a situation where resolutions, when manually set, don't work, you can open a new issue with a reproduction - but I advise you to double-check your setup against this repo to make sure you're using the correct syntax.

@yarnpkg yarnpkg locked as off-topic and limited conversation to collaborators Jun 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working upholded Real issues without formal reproduction
Projects
None yet
Development

No branches or pull requests