Yarn: Bug: "resolutions" trumps `--ignore-optional`

[joscha]

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

bug

What is the current behavior?

Currently, when there are resolutions defined, --ignore-optional is ignored and the optional dependencies are installed anyway.

You can test it with a simple package.json:

{
  "name": "test-optional",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "optionalDependencies": {
    "fsevents": "^1.0.0"
  },
  "resolutions": {
    "fsevents": "^1.2.4"
  }
}

on yarn install --ignore-optional the above will lead to fsevents being installed nonetheless.

On Linux (Ubuntu Xenial) for example, this would fail the yarn install due to:

error [email protected]: The platform "linux" is incompatible with this module.
error Found incompatible module
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

yarn install --ignore-optional works as expected with:

{
  "name": "test-optional",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "optionalDependencies": {
    "fsevents": "^1.0.0"
  }
}

and does not install fsevents.

What is the expected behavior?

No optional dependencies to be installed with --ignore-optional, no matter whether they are mentioned in resolutions or not.

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

yarn 1.7.0
Node 10.5.0
OSX High Sierra and Debian Xenial

[Gudahtt]

I was able to reproduce this problem. If fsevents is an optional dependency (direct or transitive) and is specified in resolutions, the install fails on non-macOS platforms. It even fails without the --ignore-optional flag.

It's not clear on what the expected behavior is for optional dependencies specified in resolutions. Intuitively I'd expect the version set in resolutions to be ignored if the optional package fails to install, but I can't find that specified in the selective-versions-resolutions RFC.

[deftomat]

The same for me with yarn 1.9.4.
I'm pretty sure that the expected behaviour for resolutions is JUST to override a package version. Not to force their installation. Current behaviour is unusable when you have developers with different environments 😞

[wycats]

I hit this problem too, and agree that this behavior is unexpected. Is there even a workaround?

[rally25rs]

Took some time to start digging into this. Trying to understand exactly how resolutions works, but it looks like this line https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/install.js#L286 makes a pkg that is hard-coded to optional: false which is then used to replace another dependency when it matches the resolution.

This happens before the actual dependencies are matched to resolutions, so the above line can't just be altered to match the actual dependency's optional property.

I haven't found the exact spot yet, but I'm guessing that at some point it's taking the fsevents@^1.0.0 optional: true reference and replacing it with the already-resolved fsevents@^1.2.4 optional: false.

If anyone has time to jump in and help, it would be appreciated...

[jasonxia23]

Would like to take a try.

[parris]

I think I found a possible workaround (that may work in some cases - worked for me with chokidar and fsevents) - install with the package's version resolution on a compatible environment. Then remove the resolution and yarn install again. Commit all that. The only problem with this is needing to redo the resolution from time to time.

[ghost]

@joscha I have similar problem. Did you find any good work around for it?

[joscha]

@joscha I have similar problem. Did you find any good work around for it?

I did not unfortunately, sorry.

[coreyward]

I suspect it has something to do with line 289, where optional: false is hardcoded:

https://github.com/yarnpkg/yarn/blob/ee7b6c6a5b552f4d6ef66dfe95db33bef71511cf/src/cli/commands/install.js#L287-L291

This behavior was introduced by @kaylie-alexa (committed by @arcanis) in #4105. Perhaps one of them can shed some light on how to proceed with allowing resolutions to co-exist with optional dependencies?

[kaylie-alexa]

So I looked into the issue, and the real problem is that the deduping function is grouping the two dependency types together if their semver ranges match. So for example if you have

 {
  "name": "test-optional",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "optionalDependencies": {
    "fsevents": "1.0.0"
  },
  "resolutions": {
    "fsevents": "^1.2.4" // resolves to 1.2.9
}

you'll notice that the --ignore-optional flag will work as intended. I'll look into an actual fix, but that'd be the temporary workaround.

[olingern]

@kaylie-alexa I started looking at this, too. Maybe we can compare notes. Here's where I'm at:

• Checking validity via removing and adding resolutions in a test project. ( slightly painful :cry: )
• Logging my tears away

Basically, I'm just:

• rm -rf ./node_modules
• Running ./bin/yarn --ignore-optional
• Isolating differences between a package.json with resolutions and without.

With this package.json:

{
  "name": "yarn_testing",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "optionalDependencies": {
    "left-pad": "^1.3.0"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "right-pad": "^1.0.1"
  },
  "resolutions": {
    "left-pad": "^1.3.0"
  }
}

---

Fix

Ensure resolutions do not conflict with optional dependencies

I was able to verify that resolutions make a package required, so even when --ignore-optional is specified, you'll see optional dependencies marked as required in visibleFlatTree that's returned from PackageHoister.init, which is used to install dependencies. So, when a resolution is specified you get something like this:

    HoistManifest {
      isDirectRequire: true,
      isRequired: true,
      isIncompatible: false,
      loc: '/home/olingern/.cache/yarn/v4/npm-left-pad-1.3.0-5b8a3a7765dfe001261dde915589e782f8c94d1e/node_modules/left-pad',
      pkg: [Object],
      key: 'left-pad',
      parts: [Array],
      originalKey: 'left-pad',
      previousPaths: [],
      history: [Array],
      isNohoist: false,
      originalParentPath: '',
      shallowPaths: [],
      isShallow: false,
      nohoistList: null } ],

isRequired for the package left-pad should be false!

Digging into isRequired

Getting to it was tricky, but with some pain I tracked it down:

• linker.init ( _I thought this was the right place to start, ha._ )
• copyModules
• getFlatHoistedTree
• hoister.seed
• _seed
• isRequired

Some debugging output for good measures around this line. Here's the debug code I used:

    console.log("\n")
    console.log("--------------------------------------")
    console.log(pattern + " isRequired --> " + isRequired)
    console.log("--------------------------------------")
    console.log("ref.optional \t\t | \t" + ref.optional);
    console.log("this.ignoreOptional \t | \t" + this.ignoreOptional);
    console.log("isDirectRequire \t | \t" + isDirectRequire);
    console.log("!ref.ignore \t\t | \t" + !ref.ignore);
    console.log("!isIncompatible \t | \t" + !ref.ignore);
    console.log("!isMarkedAsOptional \t | \t" + !isMarkedAsOptional);

No resolutions:

--------------------------------------
right-pad@^1.0.1 isRequired --> true
--------------------------------------
ref.optional             |      false
this.ignoreOptional      |      true
isDirectRequire          |      true
!ref.ignore              |      true
!isIncompatible          |      true
!isMarkedAsOptional      |      true


--------------------------------------
left-pad@^1.3.0 isRequired --> false
--------------------------------------
ref.optional             |      true   <-- correct
this.ignoreOptional      |      true
isDirectRequire          |      true
!ref.ignore              |      true
!isIncompatible          |      true
!isMarkedAsOptional      |      true

With resolutions:

--------------------------------------
right-pad@^1.0.1 isRequired --> true
--------------------------------------
ref.optional             |      false
this.ignoreOptional      |      true
isDirectRequire          |      true
!ref.ignore              |      true
!isIncompatible          |      true
!isMarkedAsOptional      |      true


--------------------------------------
left-pad@^1.3.0 isRequired --> true
--------------------------------------
ref.optional             |      false   <-- not correct
this.ignoreOptional      |      true
isDirectRequire          |      true
!ref.ignore              |      true
!isIncompatible          |      true
!isMarkedAsOptional      |      true

---

Eventually, I arrived back at exactly where @coreyward points out that this could be a problem. It's due to actually returning two entries of the same package, one optional and one not.

    return {
      requests: [...resolutionDeps, ...deps],
      patterns,
      manifest,
      usedPatterns,
      ignorePatterns,
      workspaceLayout,
    };

If we log what is being returned here, we'll find this:

=====================
  resolutionDeps
=====================
[ { registry: 'npm',
    pattern: 'left-pad@^1.3.0',
    optional: false,
    hint: 'resolution' } ]
=====================
  deps
=====================
[ { pattern: 'right-pad@^1.0.1',
    registry: 'npm',
    hint: null,
    optional: false,
    workspaceName: 'yarn_testing',
    workspaceLoc: undefined },
  { pattern: 'left-pad@^1.3.0',
    registry: 'npm',
    hint: 'optional',
    optional: true,
    workspaceName: 'yarn_testing',
    workspaceLoc: undefined } ]

---

PR

I opened #7272 as a place to talk about the implementation.

[kaylie-alexa]

Thanks so much for sleuthing @olingern! You're right resolutions shouldn't always default to being optional. I posted a PR to address the issue.

[xMort]

When was this released? I just encountered this behavior with fsevents and yarn 1.22.4

[seanmakesgames]

Took a look at the fix and it looks like the 'checking for if it's marked as optional' is happening at the same package, not looking at the fully resolved subdependencies' packages optional status
https://github.com/yarnpkg/yarn/commit/b6569538de69e0ccd201f0a33f1f5b52f2656f5b#diff-ffb8cf10e4b2d16cc7df603574698af6R289
If you aren't marking the dependency as optional in the top level package.
Additionally, you must specify --ignore-optional to get it to work.

Any of you thumbs-uppers from the comment above (march 24th) in the same situation as us?

[bdurrer]

@seanmakesgames Sadly yes, I get the error with yarn 1.22.5 on windows using the configuration below, unless I specifiy --ignore-optional. (Several versions of fsevents 1.x.x are referenced by sub-dependencies too)

  "optionalDependencies": {
    "fsevents": "1.2.13"
  },
  "resolutions": {
    "fsevents": "1.2.13",
    "**/fsevents": "1.2.13"
  },