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

Fixing dependencies to support yarn2 strict mode #6942

Closed
wants to merge 1 commit into from
Closed

Fixing dependencies to support yarn2 strict mode #6942

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
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,6 @@ benchmark/dist
# Docs preview
docs/_loopback.io/
docs/_preview/

# IDE files
/.idea
Copy link
Member

Choose a reason for hiding this comment

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

I believe we are intentionally not listing .idea files in .gitignore, advising users to setup a global ignore rule instead.

If you think it's time to change that, then please open a new pull request where we can discuss the change on it's own, independently on feature/bug fix work.

2 changes: 1 addition & 1 deletion packages/cli/generators/openapi/spec-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

'use strict';
const chalk = require('chalk');
const SwaggerParser = require('swagger-parser');
const SwaggerParser = require('@apidevtools/swagger-parser');
const swagger2openapi = require('swagger2openapi');
const {debugJson, cloneSpecObject} = require('./utils');
const {generateControllerSpecs} = require('./spec-helper');
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"access": "public"
},
"dependencies": {
"@apidevtools/swagger-parser": "^10.0.2",
Copy link
Member

Choose a reason for hiding this comment

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

Any reason/benefit for the switch? AFAIK, both are actively maintained as aliases.

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 not mix multiple kinds of changes in a single pull request please.

Either switch the swagger parser implementation, or fix dependencies to enable yarn2, but don't do both.

"@lerna/project": "^3.21.0",
"@openapi-contrib/openapi-schema-to-json-schema": "^3.0.4",
"@phenomnomnominal/tsquery": "^4.1.1",
Expand All @@ -19,6 +20,7 @@
"debug": "^4.3.1",
"fs-extra": "^9.0.1",
"glob": "^7.1.6",
"inquirer": "^7.3.3",
"inquirer-autocomplete-prompt": "^1.3.0",
"json5": "^2.1.3",
"latest-version": "^5.1.0",
Expand All @@ -27,6 +29,7 @@
"minimist": "^1.2.5",
"mkdirp": "^1.0.4",
"natural-compare": "^1.4.0",
"openapi-types": "^7.0.1",
"pacote": "^11.1.13",
"pluralize": "^8.0.0",
"regenerate": "^1.4.2",
Expand All @@ -35,7 +38,6 @@
"spdx-license-list": "^6.3.0",
"stringify-object": "^3.3.0",
"strong-globalize": "^6.0.5",
"swagger-parser": "^10.0.2",
"swagger2openapi": "^7.0.4",
"tabtab": "^3.0.2",
"terminal-link": "^2.1.1",
Expand Down
1 change: 1 addition & 0 deletions packages/openapi-v3/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"@loopback/core": "^2.13.1"
},
"dependencies": {
"@loopback/repository": "^3.3.0",
Comment on lines 26 to +29
Copy link
Member

Choose a reason for hiding this comment

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

Would Yarn2 PnP be satisfied with it being referenced as a peerDependency instead?

The reason for peerDependency is so that only a single version of the package is used across the project and its dependencies (See #5959). Adding @loopback/repository as a hard dependency would break this paradigm.

Though it got me curious if there's any other LB4 packages that have the same issue.

We also probably should update our CI pipeline to test with Yarn2 PnP.

Suggested change
"@loopback/core": "^2.13.1"
},
"dependencies": {
"@loopback/repository": "^3.3.0",
"@loopback/core": "^2.13.1"
"@loopback/repository": "^3.3.0"
},
"dependencies": {

Copy link
Member

Choose a reason for hiding this comment

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

-1. Our extensions are intentionally using peer dependencies for @loopback/repository, as explained in the comment above.

To fix the problem you are observing with Yarn2 PnP, we should find the place that's depending on @loopback/openapi-v3 but not fulfilling it's peer dependency on @loopback/repository. I think we need to add @loopback/repository to peerDependencies of @loopback/rest.

https://github.com/strongloop/loopback-next/blob/81fab94a16f09da8d6251cef68c33b127861c0d4/packages/rest/package.json#L26-L32

Copy link
Member

Choose a reason for hiding this comment

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

We also probably should update our CI pipeline to test with Yarn2 PnP.

That's a great idea 👍 Would you @achrinza mind opening a new GH issue for that?

"@loopback/repository-json-schema": "^3.2.0",
"debug": "^4.3.1",
"http-status": "^1.5.0",
Expand Down