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.

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

NPM/PNPM: Add support for peer dependencies #95

Closed
mnonnenmacher opened this issue Dec 1, 2017 · 2 comments · Fixed by #7785
Closed

NPM/PNPM: Add support for peer dependencies #95

mnonnenmacher opened this issue Dec 1, 2017 · 2 comments · Fixed by #7785
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements

Comments

@mnonnenmacher
Copy link
Member

mnonnenmacher commented Dec 1, 2017 •
edited
Loading

Add support for the peerDependencies scope to NPM. Be aware that the specific version of the peer dependency will only be resolved in the context of a project using the library.

npm-install-peers can be used to install peer dependencies, see this relevant ticket:
spatie/npm-install-peers#13
@mnonnenmacher mnonnenmacher added analyzer About the analyzer tool enhancement Issues that are considered to be enhancements labels Dec 1, 2017
@tsteenbe
Copy link
Member

@mnonnenmacher Harp.gl has multiple peer dependency on three.js see https://github.com/heremaps/harp.gl/blob/e3110f1dc35a889059ba23faefc76e3a4c1cfe73/%40here/harp-debug-datasource/package.json#L42

Now ORT does not pick up the core library that powers harp.gl

@sschuberth sschuberth added the hacktoberfest An issue suitable for the Hacktoberfest label Oct 2, 2021
@sschuberth
Copy link
Member

sschuberth commented Nov 3, 2021 •
edited
Loading

Good write-up about what peer dependencies actually are: https://nodejs.org/en/blog/npm/peer-dependencies/. Basically, peer dependencies define against what API of a "host" package a (plugin) package has been written.

A problem is that different NPM versions behave differently WRT to peer dependencies, quoting the article:

UPDATE: npm versions 1, 2, and 7 will automatically install peerDependencies if they are not explicitly depended upon higher in the dependency tree. For npm versions 3 through 6, you will receive a warning that the peerDependency is not installed instead.

sschuberth added a commit that referenced this issue Nov 4, 2021
@sschuberth
analyzer: Remove peerDependencies from NPM / Yarn test assets
76ce89e 
ORT does not support `peerDependencies` yet [1]. That is mostly because
handling of peer dependencies varies with NPM versions: Some versions do
install them by default, some do not [2]. This results in ORT's functional
tests to fail depending on the NPM version, and as it turn out, they
would fail with the NPM version 7 currently used in ORT's `Dockerfile`.
Avoid that by simply not using peer dependencies for testing for now.

[1]: #95
[2]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
sschuberth added a commit that referenced this issue Nov 4, 2021
@sschuberth
analyzer: Remove peerDependencies from NPM / Yarn test assets
2eef5e2 
ORT does not support `peerDependencies` yet [1]. That is mostly because
handling of peer dependencies varies with NPM versions: Some versions do
install them by default, some do not [2]. This results in ORT's functional
tests to fail depending on the NPM version, and as it turn out, they
would fail with the NPM version 7 currently used in ORT's `Dockerfile`.
Howver, tests succeed on Azure CI which uses NPM 6. Avoid that issue by
simply not using peer dependencies for testing for now.

[1]: #95
[2]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
@sschuberth sschuberth mentioned this issue Nov 4, 2021
Merged
sschuberth added a commit that referenced this issue Nov 4, 2021
@sschuberth
analyzer: Remove peerDependencies from NPM / Yarn test assets
e2945f7 
ORT does not support `peerDependencies` yet [1]. That is mostly because
handling of peer dependencies varies with NPM versions: Some versions do
install them by default, some do not [2]. This results in ORT's functional
tests to fail depending on the NPM version, and as it turn out, they
would fail with the NPM version 7 currently used in ORT's `Dockerfile`.
However, tests succeed on Azure CI which uses NPM 6. Avoid that
inconsistency by simply not using peer dependencies at all for testing for
now.

[1]: #95
[2]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
sschuberth added a commit that referenced this issue Nov 5, 2021
@sschuberth
analyzer: Remove peerDependencies from NPM / Yarn test assets
e19ba34 
ORT does not support `peerDependencies` yet [1]. That is mostly because
handling of peer dependencies varies with NPM versions: Some versions do
install them by default, some do not [2]. This results in ORT's functional
tests to fail depending on the NPM version, and as it turns out, they
would fail with the NPM version 7 currently used in ORT's `Dockerfile`.
However, tests succeed on Azure CI which uses NPM 6. Avoid that
inconsistency by simply not using peer dependencies at all for testing for
now.

[1]: #95
[2]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
sschuberth added a commit that referenced this issue Nov 5, 2021
@sschuberth
analyzer: Remove peerDependencies from NPM / Yarn test assets
ab292d9 
ORT does not support `peerDependencies` yet [1]. That is mostly because
handling of peer dependencies varies with NPM versions: Some versions do
install them by default, some do not [2]. This results in ORT's functional
tests to fail depending on the NPM version, and as it turns out, they
would fail with the NPM version 7 currently used in ORT's `Dockerfile`.
However, tests succeed on Azure CI which uses NPM 6. Avoid that
inconsistency by simply not using peer dependencies at all for testing for
now.

[1]: #95
[2]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
@sschuberth sschuberth removed the hacktoberfest An issue suitable for the Hacktoberfest label Jan 20, 2022
@MarcelBochtler MarcelBochtler changed the title NPM: Add support for peer dependencies NPM/PNPM: Add support for peer dependencies Jul 26, 2022
sschuberth added a commit that referenced this issue Nov 1, 2023
@sschuberth
docs(Npm): Do not say to implement dedicated support for peer depende…
5d94457 
…ncies

NPM's peer dependencies are not regular dependencies in a "peer" scope,
but 'Some way of saying, "I only work when plugged in to version 1.2.x of
my host package, so if you install me, be sure that it's alongside a
compatible host."' [1]. So they are more like what Gradle would call a
"dependency constraint" [2].

As any recent version of NPM "will automatically install peerDependencies
if they are not explicitly depended upon higher in the dependency tree"
[3], ORT's NPM implementation will pick them up when parsing the
installed modules, so no further handling is needed.

Resolves #95.

[1]: https://nodejs.org/en/blog/npm/peer-dependencies#the-solution-peer-dependencies
[2]: https://docs.gradle.org/current/userguide/dependency_management_terminology.html#sub:terminology_dependency_constraint
[3]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
@sschuberth sschuberth mentioned this issue Nov 1, 2023
Merged
sschuberth added a commit that referenced this issue Nov 1, 2023
@sschuberth
docs(Npm): Do not say to implement dedicated support for peer depende…
83c6477 
…ncies

NPM's peer dependencies are not regular dependencies in a "peer" scope,
but 'Some way of saying, "I only work when plugged in to version 1.2.x of
my host package, so if you install me, be sure that it's alongside a
compatible host."' [1]. So they are more like what Gradle would call a
"dependency constraint" [2].

As any recent version of NPM "will automatically install peerDependencies
if they are not explicitly depended upon higher in the dependency tree"
[3], ORT's NPM implementation will pick them up when parsing the
installed modules, so no further handling is needed.

Resolves #95.

[1]: https://nodejs.org/en/blog/npm/peer-dependencies#the-solution-peer-dependencies
[2]: https://docs.gradle.org/current/userguide/dependency_management_terminology.html#sub:terminology_dependency_constraint
[3]: https://nodejs.org/en/blog/npm/peer-dependencies#using-peer-dependencies

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs(Npm): Do not say to implement dedicated support for peer dependencies oss-review-toolkit/ort
3 participants
@sschuberth @tsteenbe @mnonnenmacher