Skip to content

fix(install): Inherit optional property for resolutions #7273

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2019
Merged

fix(install): Inherit optional property for resolutions #7273

merged 1 commit into from
Jun 6, 2019

Conversation

kaylie-alexa
Copy link
Member

Fixes #6040

Summary
Resolutions are a required for installing when there is a matching pattern in the resolutions map. When the resolver tries to dedupe dependencies and finds an optionalDependency that matches a resolution pattern, they merge into one no-longer optional request. This PR has resolutions inherit optional property if the dependency exists in the top level package.json.

Test plan

I've added a test case under resolutions

@kaylie-alexa kaylie-alexa requested a review from arcanis May 14, 2019 16:18
@olingern olingern mentioned this pull request May 14, 2019
Closed
6 tasks
@kaylie-alexa kaylie-alexa requested a review from rally25rs May 14, 2019 23:05
@coreyward
Copy link

@rally25rs @arcanis Any guidance on when we might get this reviewed and merged?

arcanis
arcanis approved these changes Jun 6, 2019
View reviewed changes
Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, sorry for the delay!

@arcanis arcanis merged commit b656953 into yarnpkg:master Jun 6, 2019
@nddery nddery mentioned this pull request Jun 10, 2019
Merged
9 tasks
@BYK BYK mentioned this pull request Jul 8, 2019
Open
robyww added a commit to Caltech-IPAC/firefly that referenced this pull request Jan 29, 2020
@robyww
Firefly-451: package.json updates
045a0e6 
  - Allows any version of node v8 or later
  - Most packages updated
  - Packages now locked at their revisions instead of minor releases
  - Babel updated, to most recent, 7.8
  - ESlint updated to most recent, and now better supports ES2020
  - Jest update some, can go to most recent with node 8
  - JSDoc updated to most recent
  - Webpack updated a few revisions, (minor)
  - Other package updated when there is compatibility
  - Cleaned out several unused packages
  - Added `package-json-doc.md` to help better document package.json
  - transformation-matrix-js moved from npm package to externalSource
     - it is not longer supported as a npm package.
  - Fixes Firefly-476
  - changed yarn install to yarn install --ignore-platform
     - this is because of a bug in yarn fsevents is an optional dependency
       and if in resolutions, npm works correctly
       see yarnpkg/yarn#7273
  - changed a couple of files of use `?.` to make sure it works
     - optional chaining
  - to update all the url load we have to change the way we reference images
    in css files. we not reference them under ~images instread of /images
robyww added a commit to Caltech-IPAC/firefly that referenced this pull request Jan 30, 2020
@robyww
Firefly-451: package.json updates
55a10a6 
  - Allows any version of node v8 or later
  - Most packages updated
  - Packages now locked at their revisions instead of minor releases
  - Babel updated, to most recent, 7.8
  - ESlint updated to most recent, and now better supports ES2020
  - Jest update some, can go to most recent with node 8
  - JSDoc updated to most recent
  - Webpack updated a few revisions, (minor)
  - Other package updated when there is compatibility
  - Cleaned out several unused packages
  - Added `package-json-doc.md` to help better document package.json
  - transformation-matrix-js moved from npm package to externalSource
     - it is not longer supported as a npm package.
  - changed yarn install to yarn install --ignore-platform
     - this is because of a bug in yarn fsevents is an optional dependency
       and if in resolutions, npm works correctly
       see yarnpkg/yarn#7273
  - changed a couple of files of use `?.` to make sure it works
     - optional chaining
  - to update all the url load we have to change the way we reference images
    in css files. we not reference them under ~images instread of /images
  - Small bug fixes:
      - Fixes Firefly-470
      - Fixes Firefly-476
      - Fixes Firefly-477
robyww added a commit to Caltech-IPAC/firefly that referenced this pull request Jan 31, 2020
@robyww
Firefly-451: package.json updates
e74dee3 
  - Allows any version of node v8 or later
  - Most packages updated
  - Packages now locked at their revisions instead of minor releases
  - Babel updated, to most recent, 7.8
  - ESlint updated to most recent, and now better supports ES2020
  - Jest update some, can go to most recent with node 8
  - JSDoc updated to most recent
  - Webpack updated a few revisions, (minor)
  - Other package updated when there is compatibility
  - Cleaned out several unused packages
  - Added `package-json-doc.md` to help better document package.json
  - transformation-matrix-js moved from npm package to externalSource
     - it is not longer supported as a npm package.
  - changed yarn install to yarn install --ignore-platform
     - this is because of a bug in yarn fsevents is an optional dependency
       and if in resolutions, npm works correctly
       see yarnpkg/yarn#7273
  - changed a couple of files of use `?.` to make sure it works
     - optional chaining
  - to update all the url load we have to change the way we reference images
    in css files. we not reference them under ~images instread of /images
  - Small bug fixes:
      - Fixes Firefly-470
      - Fixes Firefly-476
      - Fixes Firefly-477
robyww added a commit to Caltech-IPAC/firefly that referenced this pull request Feb 4, 2020
@robyww
Firefly-451: package.json updates
dcdee7b 
  - Allows any version of node v8 or later
  - Most packages updated
  - Packages now locked at their revisions instead of minor releases
  - Babel updated, to most recent, 7.8
  - ESlint updated to most recent, and now better supports ES2020
  - Jest update some, can go to most recent with node 8
  - JSDoc updated to most recent
  - Webpack updated a few revisions, (minor)
  - Other package updated when there is compatibility
  - Cleaned out several unused packages
  - Added `package-json-doc.md` to help better document package.json
  - transformation-matrix-js moved from npm package to externalSource
     - it is not longer supported as a npm package.
  - changed yarn install to yarn install --ignore-platform
     - this is because of a bug in yarn fsevents is an optional dependency
       and if in resolutions, npm works correctly
       see yarnpkg/yarn#7273
  - changed a couple of files of use `?.` to make sure it works
     - optional chaining
  - to update all the url load we have to change the way we reference images
    in css files. we not reference them under ~images instread of /images
  - Small bug fixes:
      - Fixes Firefly-470
      - Fixes Firefly-476
      - Fixes Firefly-477
robyww added a commit to Caltech-IPAC/firefly that referenced this pull request Feb 4, 2020
@robyww
Firefly-451: package.json updates
0a339dd 
  - removed all reference to numeral package
  - Allows any version of node v8 or later
  - Most packages updated
  - Packages now locked at their revisions instead of minor releases
  - Babel updated, to most recent, 7.8
  - ESlint updated to most recent, and now better supports ES2020
  - Jest update some, can go to most recent with node 8
  - JSDoc updated to most recent
  - Webpack updated a few revisions, (minor)
  - Other package updated when there is compatibility
  - Cleaned out several unused packages
  - Added `package-json-doc.md` to help better document package.json
  - transformation-matrix-js moved from npm package to externalSource
     - it is not longer supported as a npm package.
  - changed yarn install to yarn install --ignore-platform
     - this is because of a bug in yarn fsevents is an optional dependency
       and if in resolutions, npm works correctly
       see yarnpkg/yarn#7273
  - changed a couple of files of use `?.` to make sure it works
     - optional chaining
  - to update all the url load we have to change the way we reference images
    in css files. we not reference them under ~images instread of /images
  - respond from feedback
  - Small bug fixes:
      - Fixes Firefly-470
      - Fixes Firefly-476
      - Fixes Firefly-477
starrify added a commit to starrify/puppeteer-extra that referenced this pull request Mar 9, 2020
@starrify
Adding explicit declaration of fsevents as an optional dependency in
3f7d829 
package.json, therefore it may be processed properly when there's
--ignore-optional (thanks to yarnpkg/yarn#7273).
Saves GNU/Linux users from the incompatible platform issue during
installation.
Also regenerated yarn.lock after the change.
@starrify starrify mentioned this pull request Mar 9, 2020
Merged
VincentBailly pushed a commit to VincentBailly/yarn that referenced this pull request Jun 10, 2020
@kaylie-alexa
fix(install): Inherit optional property for resolutions (yarnpkg#7273)
895055c 
6040
VincentBailly pushed a commit to VincentBailly/yarn that referenced this pull request Jun 10, 2020
@kaylie-alexa @VincentBailly
fix(install): Inherit optional property for resolutions (yarnpkg#7273)
ba8d099 
6040
@eps1lon eps1lon mentioned this pull request Feb 15, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcanis arcanis arcanis approved these changes

@rally25rs rally25rs Awaiting requested review from rally25rs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: "resolutions" trumps --ignore-optional
3 participants
@kaylie-alexa @coreyward @arcanis