feat: add Yarnrc component for YARN_BERRY #3048
Conversation
There was a problem hiding this comment.
@mrgrain - I had some time today to work on this. Could you take a look at the inline questions on this draft PR review? Once I get confirmation on how you'd like to proceed, I can finish this up and get it ready for review.
TYIA!
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3048 +/- ##
==========================================
+ Coverage 96.34% 96.40% +0.06%
==========================================
Files 185 186 +1
Lines 34566 35090 +524
Branches 3208 3244 +36
==========================================
+ Hits 33302 33828 +526
+ Misses 1264 1262 -2
☔ View full report in Codecov by Sentry. |
|
|
||
| private configureYarnBerry(project: Project, options: NodePackageOptions) { | ||
| const { | ||
| version = "4.0.1", |
There was a problem hiding this comment.
corepack requires a fully qualified version for the packageManager property. If you provide values like yarn@4, yarn@stable, yarn@~4, etc. you get this error:
Usage Error: Invalid package manager specification in package.json; expected a semver version
$ yarn ...
So, unfortunately, we have to specify a full version here.
There was a problem hiding this comment.
Ugh. That's annoying. How about we make the version a required option?
There was a problem hiding this comment.
How would that work if people didn't provide yarnBerryOptions at all? e.g.,
const project = new javascript.NodeProject({
// ...
packageManager: javascript.NodePackageManager.YARN_BERRY,
});Would we throw a runtime error? I'm not sure if that'll be annoying to folks.
I suppose we could introduce it only for YARN_BERRY and not YARN2. YARN2 could not write a .yarnrc.yml file, giving people more time to transition.
There was a problem hiding this comment.
Yeah that's annoying. I think we might be able to turn javascript.NodePackageManager into an enum like class. With static props for the existing values and YARN_BERRY() a static method that requires a version?
Which feels like what we would really need to do for all corepack package managers?! So we'd end up having an interface like this:
const project = new javascript.NodeProject({
// Corpack
packageManager: javascript.CorePack.yarn('1.23.1'),
packageManager: javascript.CorePack.yarn('4.0.1'),
packageManager: javascript.CorePack.pnpm('8.0.0'),
packageManager: javascript.CorePack.npm('100.0'),
// pre-installed
packageManager: javascript.NodePackageManager.NPM,
packageManager: javascript.NodePackageManager.PNPM,
packageManager: javascript.NodePackageManager.YARN_CLASSIC,
packageManager: javascript.NodePackageManager.YARN_BERRY,
});The pre-installed ones maybe just all need to be deprecated right away? Or at least for YARN_BERRY which recommends corepack.
But let's do this separately. Happy to keep the default version for now.
There was a problem hiding this comment.
Sounds good - I filed #3070 to track that. FYI, npm is the only one that cannot be managed by corepack (lol). If we're cool leaving the default version for now, this is ready for review & merge.
There was a problem hiding this comment.
Yep. I'll give it a final review tomorrow.
|
Btw, although this isn't directly related, we might be getting some nice Corepack reliability improvements in berry: yarnpkg/berry#5912 |
Fixes issue introduced with #3048. We should allow people to set potentially conflicting values to the same value. Previously, I was just checking to see if they were both set at the same time. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
This PR significantly improves
projen's ability to use Yarn Berry.Relates to #2980
Known Limitations
pluginsyet. When you runyarn plugin import <plugin name>, it writes to.yarnrc.ymland stores the plugin code in.yarn. We'll have to figure out the best way to handle that. The good news is that Yarn v4 includes the most common plugins by default again.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.