nodejs
[WIP] Add support for bun
Work in progress - adding support for Bun. I filled out the json as best I see fit.
Investigating tests next
Closes #295
I don't know why CI is failling at basic install step. Works fine locally for me
I don't know why CI is failling at basic install step. Works fine locally for me
Looks like those tests are trying to install yarn? Wonder how that could be your fault...
Node.js v20.6 changed the ESM loader a bit so the version of Yarn used by Corepack needs to be updated to a version containing https://github.com/yarnpkg/berry/pull/5677.
Fixed by https://github.com/nodejs/corepack/pull/308.
Would this be a symlink that does not execute JS in node? I think we should have a hard requirement that if Bun is to be included in Corepack, it must not incur the startup cost penalty of Node before executing bun (outside of the initial run to lazily install it)
I've seen cases where, for example, pnpm starts 2x slower due to using the executable via corepack instead of the pnpm executable
All calls to all commands have to go through Corepack, since it has to read the packageManager field to know the version of the package manager to use. It doesn't "cache" this information.
@Ethan-Arrowood is it something you're still working on? Is this block on something?
Hey, no I'm not actively working on this anymore. It looks like we won't get sign off from Bun unless core pack is modified (https://github.com/nodejs/corepack/pull/307#issuecomment-1718747222). I'm happy to help move this along if there is a clear path forward though.
FWIW Corepack focuses on improving DX rather than performance, so IMO it wouldn't make sense to have a hard requirement as suggested in https://github.com/nodejs/corepack/pull/307#issuecomment-1718747222. Furthermore I don't think we need Bun permission (although if they feel strongly that we should not do that, it would be rude to not listen, but IIUC it's not what https://github.com/nodejs/corepack/pull/307#issuecomment-1718747222 is saying, please correct me if I'm wrong), so if the community wants Bun in Corepack, that's all that's needed (and passing CI of course). Contributions to improve Corepack performance are of course welcome, but it's not blocking as far as Corepack is concerned.
The path forward is to mark the PR as ready for reviews, and since the tests are already passing, get at least one approval and no objections.
Okay, I'll mark as ready for review now. And let folks review/approve/reject as they see fit.
I think this pr may need some tests of its own so if any core pack maintainers could help me with that. It would be appreciated
@merceyz can you help me with the tests? I don't understand what to add where
Hey friends, any update on corepack support for bun? Really looking forward to this. Thanks!
@merceyz can you help me with the tests? I don't understand what to add where
@Ethan-Arrowood you can use https://github.com/nodejs/corepack/pull/333 for inspiration, I think we just need tests that validates the download succeeds and bun is able to run.
Bun package on the npm registry is relying on optional dependencies (https://www.npmjs.com/package/bun?activeTab=code). Because Corepack is not a package manager, it has no support for dependencies, so it doesn't work. Worth noting the bun binary tries to spawn a subprocess that expects npm to be on the $PATH, which seems problematic in the context of Corepack – I would consider it a bug if Corepack needed npm to be installed on the PATH to operate.
Support for Bun is still possible to implement into Corepack, but probably not from the npm package.
Note you also need to update the following list: https://github.com/nodejs/corepack/blob/12f1c31a0decb867f91f31ffb0cadf34ce9f0673/sources/types.ts#L4-L8
I'm not sure I understand, so we need to find a different url for installing bun? Would their install script suffice https://bun.sh/install ?
I'm also going to re-emphasize that I really don't need to be driving this anymore. If this is more complex, I'd rather the Bun folks finish the swing here.