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

fix: update peerDependencies #4937

Closed
wants to merge 1 commit into from
+9 −9
Closed

fix: update peerDependencies #4937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/booter-lb3app/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
"tslib": "^1.11.1"
},
"peerDependencies": {
"@loopback/boot": "^1.2.2",
"@loopback/core": "^1.5.0",
"@loopback/rest": "^1.10.3"
"@loopback/boot": "^1.2.2 || ^2.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I would prefer to move away from peerDependencies as it's really half-baked and not bullet-proof with npm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bajtos WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Let's discuss.

I introduced peer dependencies as a solution for two problems:

(1)
As an extension (e.g. @loopback/booter-lb3app), I want to use the core packages provided by the target application (LoopBack project). I do not want to bundle my own copy of core packages.

Why is this important? It gives the target application full control of which version of core packages are used. I am not talking about version numbers - I want to allow users to use their own fork of a core package if they have a bug fix that is not a part of an official release (yet).

(2)
As an extension, I want to specify which versions of core packages I am compatible with. This way LoopBack users can pick the right versions and if they don't, then npm install will warn them about a possible problem.

I am ok to use a different solution than peer dependencies as long as it preserves the properties of the current setup.

I see two discussion points:

  1. Do we want to continue using peerDependencies or not?
  2. If yes, then what version ranges to use in peer dependencies? (Support only a single semver-major version or support as many versions as feasible?)

Let's keep this thread for discussing the first point and discuss the second point in https://github.com/strongloop/loopback-next/pull/4937/files#r395738578 below.

Copy link
Member

Choose a reason for hiding this comment

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

peerDependencies as it's really half-baked and not bullet-proof with npm.

From what I remember, peer dependencies used to be considered as deprecated in the past, but in the last few years the situation improved and they are no longer frowned upon. Popular tools like eslint and babel are using peer dependencies for their plugins.

$ npm info eslint-config-airbnb peerDependencies
{
  eslint: '^5.16.0 || ^6.8.0',
  'eslint-plugin-import': '^2.20.1',
  'eslint-plugin-jsx-a11y': '^6.2.3',
  'eslint-plugin-react': '^7.19.0',
  'eslint-plugin-react-hooks': '^2.5.0 || ^1.7.0'
}

$ npm info @babel/preset-env peerDependencies
{
  '@babel/core': '^7.0.0-0' 
}

"@loopback/core": "^1.5.0 || ^2.0.0",
"@loopback/rest": "^1.10.3 || ^2.0.0 || ^3.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not very confident with the wide range as the CI only tests the module against latest versions of such dependencies.

The most reliable collection of @loopback/* modules that are guaranteed to work together are these being released together :-).

Copy link
Member

Choose a reason for hiding this comment

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

@raymondfeng I think users will expect peer deps that meets the folowing criteria would work:

  • Same major semver
  • Equal or higher that min. version within semver (e.g. @loopback/boot: ^1.2.2)

If there's incompatibilities between 2 versions that meet the aforementioned criteria then we are breaking semver conventions.

So it may be a good idea to keep this range instead of pinning it to an exact version.

Copy link
Contributor

Choose a reason for hiding this comment

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

@achrinza I meant to suggest we use "@loopback/rest": "^3.0.0". It's hard to ensure that this module is compatible with 1.x, 2.x, and 3.x without meaningful CI test coverage.

Copy link
Member

Choose a reason for hiding this comment

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

Take our connectors as an example. They support both LB3 and LB4, therefore we have only one version to maintain for both LB3 and LB4 users.

I think it makes a lot of sense to adopt the same approach for our extensions once we start maintaining LB4 LTS versions (see #4398). This may include writing acceptance tests to verify that the current (master) version of an extension works with older (LTS) versions of a core component, as @raymondfeng has mentioned in his comment.

However, we don't provide LTS for LB4 yet, therefore I think it's ok to use a narrow range for the peer dependencies for now. Especially considering that we don't have CI test coverage to verify compatibility with older versions. I don't think it would be a good use of our time to work on such CI setup now.

If we decide to limit the supported peer versions to the latest major version only, then can we please find a way how to automate the update process - ideally as part of our version release setup? I.e. when we publish a new version, then the peer dependencies should be updated to match the new versions created by the publishing.

},
"devDependencies": {
"@loopback/boot": "^2.0.1",
Expand Down
4 changes: 2 additions & 2 deletions packages/model-api-builder/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
"copyright.owner": "IBM Corp.",
"license": "MIT",
"peerDependencies": {
"@loopback/core": "^1.12.0",
"@loopback/repository": "^1.16.0"
"@loopback/core": "^1.12.0 || ^2.0.0",
"@loopback/repository": "^1.16.0 || ^2.0.0"
},
"devDependencies": {
"@loopback/build": "^1.7.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/repository-tests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"tslib": "^1.11.1"
},
"peerDependencies": {
"@loopback/repository": "^1.6.1"
"@loopback/repository": "^1.6.1 || ^2.0.0"
},
"files": [
"README.md",
Expand Down
6 changes: 3 additions & 3 deletions packages/rest-crud/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
"@types/node": "^10.17.17"
},
"peerDependencies": {
"@loopback/core": "^1.12.4",
"@loopback/repository": "^1.19.1",
"@loopback/rest": "^2.0.0"
"@loopback/core": "^1.12.4 || ^2.0.0",
"@loopback/repository": "^1.19.1 || ^2.0.0",
"@loopback/rest": "^2.0.0 || ^3.0.0"
},
"files": [
"README.md",
Expand Down