feat: add support for Yarn patch protocol #887
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #887 +/- ##
==========================================
+ Coverage 97.96% 97.96% +0.01%
==========================================
Files 145 145
Lines 8852 8856 +4
Branches 2224 2226 +2
==========================================
+ Hits 8671 8675 +4
Misses 180 180
Partials 1 1 ☔ View full report in Codecov by Sentry. |
| ); | ||
| const node = new PackageGraph([pkg]).get('my-pkg'); | ||
|
|
||
| console.log('node=', node); |
There was a problem hiding this comment.
we probably don't need this console.log, do we? If not please remove it.
There was a problem hiding this comment.
@ghiscoding Oops, that was left there by accident, apologies. Sending an update soon!
There was a problem hiding this comment.
@ghiscoding Removed the logging, please see the updated sources!
| // Yarn decided to ignore https://github.com/npm/npm/pull/15900 and implemented "link:" | ||
| // As they apparently have no intention of being compatible, we have to do it for them. | ||
| // @see https://github.com/yarnpkg/yarn/issues/4212 | ||
| let spec = graphDependencies[depName].replace(/^link:/, 'file:'); | ||
| let spec = graphDependencies[depName].replace(/^link:/, 'file:').replace(YARN_PATCH_PROTOCOL_REG_EXP, '$2'); |
There was a problem hiding this comment.
I wonder if this will have a perf impact? Wouldn't it be better to make sure that the substring does really start with the text "patch:", the previous check of /^link:/ does that with the use of ^ but you're not using it in your regex, so I think it might better to validate the starting text and do something like this maybe
if (spec.startsWith('patch:')) {
spec = spec.replace(YARN_PATCH_PROTOCOL_REG_EXP, '$2');
}what do you think?
There was a problem hiding this comment.
@ghiscoding I agree it's more readable and most likely more performant that way as well, thank you for the great suggestion, I updated it exactly like that!
|
@petermetz apart from the 2 comments, the concept looks good, short and sweet :) |
Transparently handles the patch protocol version syntax by falling back to the base version string. lerna-lite#223 https://yarnpkg.com/cli/patch Fixes lerna-lite#223 Signed-off-by: Peter Somogyvari <peter.metz@unarin.com>
petermetz
force-pushed
the
feat-support-yarn-patch-protocol
branch
from
5c4e706
to
413a836
Compare
@ghiscoding Thank you, I made the changes! I left the conversations unresolved: I wasn't sure if you prefer to resolve them yourself or not. |
|
Great, it looks good now, ready to merge |
@ghiscoding Nice! Thank you very much for the quick turnaround!!! |
|
all good, I wanted to push a new release by tomorrow anyway so the timing is good. |
Description
Introduced logic that transparently handles the patch protocol version syntax by falling back
to the base version string.
#223
https://yarnpkg.com/cli/patch
Fixes #223
Signed-off-by: Peter Somogyvari peter.metz@unarin.com
Motivation and Context
I have third-party packages in our monorepo that ship with issues that I need to patch in a yarn v4 build setup.
Without this change lerna-lite crashes if we are using it in a project that has any yarn patch protocol usage.
#223
How Has This Been Tested?
1.1. We made changes to third party dependencies (code in the node_modules folder)
1.2. We committed said changes with
yarn patch1.3. The build was broken
1.4. Applied this fix to lerna-lite and now the build is working again.
package-graph.spec.tsTypes of changes
Checklist: