peerDependenciesMeta support

[filipesilva]

🚀 feature request

Relevant Rules

yarn_install and npm_install

Description

A clear and concise description of the problem or missing capability...

Both yarn and npm support the peerDependenciesMeta, which can make peer dependencies optional.

Without it, packages like sass-loader will error out unless the optional peer dependencies are installed:

STDERR:
could not find peer dependency 'fibers' of 'sass-loader'
ERROR: no such package '@npm//': Traceback (most recent call last):
	File "/home/circleci/.cache/bazel/_bazel_circleci/d22d44146b8f6c1dabaff0f746a8c6b9/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl", line 404
		_create_build_files(repository_ctx, "yarn_install", node, ...)
	File "/home/circleci/.cache/bazel/_bazel_circleci/d22d44146b8f6c1dabaff0f746a8c6b9/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl", line 171, in _create_build_files
		fail(("generate_build_file.js failed:...)))

Describe the solution you'd like

If you have a solution in mind, please describe it.

Describe alternatives you've considered

Ignoring the peer dependencies (#1162) might work too.

[Toxicable]

Ahhhh awesome, thanks @filipesilva for finding this.

For ref there's eample usage here: #1163

Our aim here is to not do anything more/less strict than npm/yarn, meaning that currently since they treat unsatisfied peer deps as warnings, we should also.
This is what #1163 aims to align with.

With that in mind, I don't think we should be reading this field and acting on it ourselves, we should let yarn/npm do that logic for us. If there's some issue with the deps management then yarn/npm should be the ones to report it.

[clydin]

Neither yarn, npm, nor pnpm treat them as warnings if the peerDependenciesMeta optional flag is present. The npm registry currently has a defect which causes a warning on first install when using npm but this is in the process of being corrected.

[alexeagle]

@filipesilva can you help me understand how this differs from #1163? @bweston92 do you understand this well enough to get on the standard solution?

[bweston92]

@alexeagle I don’t unfortunately

[filipesilva]

Given these package.json entries on a library:

  "peerDependencies": {
    "node-sass": "^4.0.0"
  },
  "peerDependenciesMeta": {
    "node-sass": {
      "optional": true
    }
  },

If you install that library with either npm or yarn without also installing node-sass, neither will emit an warning. The peerDependenciesMeta key contains meta information about peer dependencies, and the optional key set to true for a peer dependency means that it is optional. Here are the tracking issues for yarn and npm.

This issue requests that yarn_install and npm_install interpret the peerDependenciesMeta key correctly when present in packages and do not error out when optional peer dependencies are missing.

#1163 add a way to ignore a list of missing packages that are defined as peer dependencies. It does not interact with the peerDependenciesMeta key or its functionality.

[bweston92]

We did something similar originally which added a new key “ignoredDependencies” we can read the “peerDependenciesMeta” instead of desired?

…

On Tue, 8 Oct 2019 at 11:01, Filipe Silva ***@***.***> wrote: Given these package.json entries on a library: "peerDependencies": { "node-sass": "^4.0.0" }, "peerDependenciesMeta": { "node-sass": { "optional": true } }, If you install that library with either npm or yarn without also installing node-sass, neither will emit an warning. The peerDependenciesMeta key contains meta information about peer dependencies, and the optional key set to true for a peer dependency means that it is optional. Here are the tracking issues for yarn <yarnpkg/yarn#6487> and npm <npm/cli#224>. This issue requests that yarn_install and npm_install interpret the peerDependenciesMeta key correctly when present in packages and do not error out when optional peer dependencies are missing. #1163 <#1163> add a way to ignore a list of missing packages that are defined as peer dependencies. It does not interact with the peerDependenciesMeta key or its functionality. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1228>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANOVWPORBM7FYM7XEDF3I3QNRLA3ANCNFSM4I4WUESQ> .

[filipesilva]

I'm not sure the usage pattern is the same. peerDependenciesMeta is a property that library authors add, whereas ignoredDependencies seems to be something consumers add.

On your example for React with TypeScript peerdeps, you wouldn't be able to use peerDependenciesMeta. The libraries you depend on would have to add it instead.

[bweston92]

Ahh sorry, yeah that leaves us with an issue of the package takes a while too add it of at all. I think we should just ignore peer dependencies in these rules personally.

…

On Tue, 8 Oct 2019 at 12:06, Filipe Silva ***@***.***> wrote: I'm not sure the usage pattern is the same. peerDependenciesMeta is a property that library authors add, whereas ignoredDependencies seems to be something consumers add. On your example for React with TypeScript peerdeps, you wouldn't be able to use peerDependenciesMeta. The libraries you depend on would have to add it instead. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1228>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANOVWJQ3ZDWBMYOIS3TZBTQNRSUZANCNFSM4I4WUESQ> .

[bweston92]

For example asking tsutils to make the typescript package optional to them would be weird on their behalf?

[filipesilva]

Depends if typescript is truly optional when using tsutils. If they have imports to typescript that will cause the lib to not function properly when typescript isn't there, then it's not really optional.

https://unpkg.com/browse/[email protected]/util/convert-ast.js shows one such import.

[clydin]

It's not used to make other package's peer dependencies optional but rather a package's own peer dependencies optional.

There are now two types of peer dependencies a package can specify:

• required peer dependencies (these are the existing peer dependencies)
• optional peer dependencies (these are defined as peer dependencies but also use the new metadata)

[bweston92]

@filipesilva create-react-app without TypeScript add-on will still have tsutils.

[filipesilva]

Sure, but that's create-react-apps problem. If they want to include deps without the matching peerdeps then those projects will have peer dependency warnings. It doesn't mean that tsutils should change its dependencies to be incorrect to suit create-react-app.

It might be an argument towards yarn_install and npm_install not erroring out and instead showing a warning though.

[alexeagle]

We just call npm/yarn. Why do we have to do anything? Don't these already support peerDependenciesMeta?
This is confusing because #1163 has tried various approaches and never been in a mergable state. I'll try to get this in so that we don't fail on missing peer deps. At that point it shouldn't be anything bazel-specific for us to do right?

[clydin]

Those optional peer dependencies could be deep in the dependency hierarchy and the end user may not even now they exist (or really should be required to know that they exist).

Ideally, Bazel should ignore optional peer dependencies that are not present and continue to error for required peer dependencies that are not present.

[mikolaj-leszczynski]

@alexeagle is there any progress in regard to this issue? I have experienced the same issue. Installing transitive dependencies helped but in long term, I can not image having to maintain in my package.json packages that I do not rely directly on.

[alexeagle]

latest release stops erroring about missing peerDeps.

Still interested in what we need to do for peerDepsMeta field

[bweston92]

imo I think these rules shouldn't care too much about peer dependencies. At least until all the libraries have declared the peerDepsMeta field otherwise we'll be back to square one.

In the case of create-react-app using ts-util I don't know how it would be possible to get around it to be honest.

[alexeagle]

okay so there is nothing more to do, it sounds like. We call the package manager and don't need to do anything but show the user what it says.