-
Notifications
You must be signed in to change notification settings - Fork 49
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.
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: allow tags in npm:x
-deps
#194
Conversation
🦋 Changeset detectedLatest commit: 958f750 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
!semver.satisfies(dependencyWorkspace.packageJson.version, range) && | ||
!range.startsWith("npm:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!semver.satisfies(dependencyWorkspace.packageJson.version, range) && | |
!range.startsWith("npm:") | |
!range.startsWith("npm:") && | |
!semver.satisfies(dependencyWorkspace.packageJson.version, range) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know about the existence of npm:
ranges and used them a couple of times... but damn, I can't find any docs that would mention them 😅
In general, we likely should relax this check more and always allow things like file:
, link:
, etc. We don't necessarily have to do it here but maybe a simple regex that would allow any protocol would do the job here? cc @emmatown do you have any opinions about this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the change you requested here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that I can't find this in the npm/yarn/pnpm docs. pnpm has a mention of this in one of examples but it doesn't even explain what it does 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we likely should relax this check more and always allow things like file:, link:, etc. We don't necessarily have to do it here but maybe a simple regex that would allow any protocol would do the job here?
This sounds good to me
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Closes #193