yarnpkg / berry

📦🐈 Active development trunk for Yarn ⚒
https://yarnpkg.com
BSD 2-Clause "Simplified" License
7.39k stars 1.11k forks source link

[Bug] yarn not ignoring optional dependencies when they are not found #2425

Closed ShirMon closed 3 years ago

ShirMon commented 3 years ago

Describe the bug

Wasn't sure if this is considered a bug or a missing feature, when adding optional dependencies in the package.json file yarn will fail installation if the dependency is not found while with npm it skips the optional dependency when not found.

To Reproduce

Add a non existing optional dependency to package,json: "optionalDependencies": { "thispackagedoesntexist": "^1.0.0" }

Screenshots

With optional dependencies: image

Without: image

Environment if relevant (please complete the following information):

arcanis commented 3 years ago

This is intended. Optional dependencies only make the build optional. If the dependency cannot be fetched, it's likely a significant error and we can't sweep it under the rug (especially since it would lead to an incorrect install if the package has a fallback for unsupported systems).

Makoehle commented 3 years ago

@arcanis I'm confused. If I don't want the build to fail due to optionalDependencies I'd personally move them to non-optional dependencies. Following your explanation the yarn-install-ignore-optional shouldn't be allowed? Imagine the use case where I have dependencies in a secure corperate infrastrucutre to fetch e.G user information which I don't require if I provide the same application outside my corperate network. The optinalDependencies flag as npm is handling it allows me to keep all dependencies in my package.json. Is there a way to use yarn for such a useCase as well?

arcanis commented 3 years ago

Imagine the use case where I have dependencies in a secure corperate infrastrucutre to fetch e.G user information which I don't require if I provide the same application outside my corperate network. Is there a way to use yarn for such a useCase as well?

No. The very purpose of the lockfile is to contain the full dependency tree, which requires to at least be able to resolve and fetch all the packages, whether they're listed as optionalDependencies or not. Without this, the lockfile would be missing entries, which would be a vulnerability (since those packages' hashes would be missing, allowing their content to change unknowingly).

Following your explanation the yarn-install-ignore-optional shouldn't be allowed?

This flag was inherited from npm, and its implications in terms of soundness and security weren't well understood at the time (in part because there was no real specification about the intended behavior).

kristian commented 2 years ago

@ShirMon and @Makoehle as my issue #4306 was closed even though, I thought I put up quite some good arguments (e.g. just referring to the package.json spec. where it clearly states that „optional“ also referrs to wether a package exists or not indeed)… I went ahead and implemented a better-optional plugin for Yarn 2+ (Berry), providing a workaround for this issue. Hope it helps you! It‘s not perfect by any strech of the imagination, but it seems to work just fine for my use case. So maybe it helps you as well! 👍 Bummer that this is not in scope for Yarn…

kristian commented 2 years ago

… since those packages' hashes would be missing, allowing their content to change unknowingly

Why not simply add null-hashes to the lock file then? If a package ceises to exist afterwards you would still know and have a clearly distiguishable state to „not in the lockfile“?

in part because there was no real specification about the intended behavior

I don‘t know about this argument… even going back to v6 of the package.json specification of NPM see [1], the specification clearly states:

If a dependency can be used, but you would like npm to proceed if it cannot be found or fails to install, then you may put it in the optionalDependencies object.

I think that makes it pretty clear what should have been the intended behavior.

[1] https://docs.npmjs.com/cli/v6/configuring-npm/package-json#optionaldependencies

freed00m commented 2 years ago

I will just drop my gitlab runner solution, where I skip-optional, sort of. If someone finds it useful.

download-dep:
  script:
    # at <projectRoot>
    # Since yarn 2 doesn't have --skip-optional => remove it with sed
    - sed -i '/\"optionalDependencies\"/,/}/ d; /^$/d' package.json

    - yarn install --no-immutable

I know yarn is opinionated, but there is need to do this to speed up CI/CDs.

I am using optionals only in case there is no need for dependency to exists inside runners.

EDIT: thx @merceyz

merceyz commented 2 years ago

enableImmutableInstalls not available as install flag

Yes it is, use --no-immutable https://yarnpkg.com/cli/install#options-immutable

vis97c commented 1 year ago

Can this implementation be reconsidered? Your own docs state certaing behavior but yarn itself does not follow it: https://classic.yarnpkg.com/lang/en/docs/dependency-types/#toc-optionaldependencies.

The null hashes or a dummy package could be a solution for the lock file requirement

kristian commented 1 year ago

Can this implementation be reconsidered? Your own docs state certaing behavior but yarn itself does not follow it: https://classic.yarnpkg.com/lang/en/docs/dependency-types/#toc-optionaldependencies.

The null hashes or a dummy package could be a solution for the lock file requirement

Actually the documentation states exactly what Yarn does right now: "If they fail to install, Yarn will still say the install process was successful.", so actually the word "fail" is taken literally here! If the dependency doesn't exist it cannot "fail", so it is violating the lockfile constraints. So "fail to install" really means "the installation fails" (e.g. a post-install script with a non-zero exit code), that is what Yarn considers "optional".

If you want a different behavior, consider switching to Yarn 2+/Berry and using my optional resolution plugin.