setup-node icon indicating copy to clipboard operation
setup-node copied to clipboard

Add automatic corepack support for node v14 +

Open Ethan-Arrowood opened this issue 3 years ago • 8 comments

Description:

Executes corepack enable when the detected node version is 14+. This will enable support for the packageManager field for yarn and pnpm

Related issue: https://github.com/actions/setup-node/issues/480

Check list:

  • [ ] Mark if documentation changes are required.
  • [x] Mark if tests were added or updated to cover the changes.

Ethan-Arrowood avatar May 02 '22 20:05 Ethan-Arrowood

Struggling to get a test to pass. Insight appreciated

Ethan-Arrowood avatar May 02 '22 20:05 Ethan-Arrowood

Probably wrong on the test, but since it is asserting the desired corepack output I'm marking this PR ready for review. Hopefully a maintainer more familiar with the test suite can provide some input on how to improve the tests.

Ethan-Arrowood avatar May 02 '22 22:05 Ethan-Arrowood

Thanks for opening this PR. I support the idea.

shellscape avatar May 04 '22 16:05 shellscape

Added a check for the packageManager field. How do I fake a package.json file for the test?

I almost want to try writing an integration test for this, but would appreciate some input from the maintainers

Ethan-Arrowood avatar May 05 '22 17:05 Ethan-Arrowood

@nickmccurdy

  1. Can you explain more about this version syntax? I’m pretty sure “corepack enable will work with nearly all versions of Yarn.
  2. I think it would be hard to remove yarn from the runner so perhaps it should be uninstalled. What’s the best way to do that? npm uninstall -g yarn?

styfle avatar Aug 28 '22 12:08 styfle

  1. Can you explain more about this version syntax? I’m pretty sure “corepack enable will work with nearly all versions of Yarn.

Oh sorry, I see what you mean. It's not actually parsing the packageManager, but rather passing it to corepack.

  1. I think it would be hard to remove yarn from the runner so perhaps it should be uninstalled. What’s the best way to do that? npm uninstall -g yarn?

GitHub Actions seems to run npm install -g yarn ..., so yea, npm uninstall -g yarn should work.

nickserv avatar Aug 28 '22 13:08 nickserv

@Ethan-Arrowood Are you planning to update this PR based on the feedback that has been provided? I know a lot of people are still interested in this feature. I see that CI is also failing currently.

jtbandes avatar Sep 27 '22 21:09 jtbandes

Looking at this PR again, it could be considered semver major because it will suddenly opt into corepack when the package.json is detected.

I'm starting to think that PR #651 is better solution because it could be considered semver minor due to the manual opt in.

That could prove valuable if anything changes in the future, since corepack is still technically experimental.

styfle avatar Apr 05 '23 15:04 styfle