Experiment: use Bun as package manager and script runner
Just experimenting with Bun to see what's possible. This removes lerna as it's no longer necessary.
Tested:
- all affected scripts (build, test, test-browser, clean, coverage, generate-images, etc)
- most untouched scripts, but run with
buninstead ofnpm(e.g.bun lint-fix - main ci tasks
- plus ci
- husky
Untested:
- changeset script
- reactnext ci
- release ci
- test-vo ci
- vercel build
- codesandbox?
To do still:
- [x] package patching
- [x]
@wordpress/componentsthing - [ ] complete testing
- [ ] update docs (contribution, etc)
- [ ] double-check everything
Future ideas:
- Replace vitest with
bun test
⚠️ No Changeset found
Latest commit: a5a0914d12cfb50e56d9f7262c554546f98b1391
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Regarding Vercel failures: https://www.vercel-status.com
@diegohaz nice timing lol, thanks for sharing. Almost done here, just need to tick off the last few items and test the remaining stuff. Any help (especially with testing) and/or thoughts about this are welcome!
The last time I tested bun test, they didn't support JSDOM, and HappyDOM wasn't as feature complete. I'm not sure how it is today.
Anyway, I'm pretty happy with Vitest. Tests will still be a bit slow because we add fake timeouts to simulate real user interaction. Unless bun can parallelize more, I don't expect a big difference.
@diegohaz thanks for the insight! To be clear, I'm not aiming for bun test in this PR, it was just an idea, but that's good to know. I think bun as pm and script runner is worth it though.
I think this file needs updating too? https://github.com/ariakit/ariakit/blob/ff2509c7d7ce75a6e45a56f1cc61302a5dc6632b/vercel.json#L1-L7
@diegohaz updated vercel.json. I can't access the build logs so let me know if it worked or if there are any errors. Same with codesandbox.
Question: is the changeset patch still needed? If so, I'll migrate it to bun's patching system. If not, we should be able to just remove it.
Separately, I could use a bit of help testing some of the remaining stuff. I'm gonna test changeset locally, but the remaining CI stuff and vercel/codesandbox are a bit trickier as a contributor, happy to do the work if you tell me how you prefer to test it though.
@diegohaz updated
vercel.json. I can't access the build logs so let me know if it worked or if there are any errors. Same with codesandbox.
There's a warning regarding the @wordpress/components override:
warn: Bun currently does not support nested "overrides"
at /vercel/path0/package.json:69:30
This override is necessary because we have tests for @wordpress/components and need to use the local version of @ariakit/react instead of the npm version that WordPress relies on.
Also, there's an error at the end of the website build, but since the site appears to build correctly, I'm unsure what it means:
website build: Exited with code 0
Question: is the changeset patch still needed? If so, I'll migrate it to bun's patching system. If not, we should be able to just remove it.
This issue is still open, so I believe the patch is still necessary: https://github.com/changesets/changesets/issues/995
Regarding CodeSandbox, I think there might be some settings that need adjusting for Bun in this file:
https://github.com/ariakit/ariakit/blob/1a279070efbb83fda9f74cb8c5677832e84750a0/.codesandbox/ci.json#L1-L6
CodeSandbox CI hasn't been very useful. I need to switch to https://pkg.pr.new at some point.
Also, leaving package-lock.json unchanged for now until this PR is merged might help avoid conflicts. Otherwise, there will be a conflict with main every time the Renovate bot merges a PR.
Re:override, wdy think about this? https://github.com/oven-sh/bun/issues/6608#issuecomment-2162028520
TL;DR: instead of nested just apply the override globally. AFAIK, this is the only case where it'll be pulled as a module, the monorepo setup and everything else should be unaffected, right?
A global override might work too. If we add more packages that depend on Ariakit, we'll probably want to do the same for them.
Also, there's an error at the end of the website build, but since the site appears to build correctly, I'm unsure what it means
Code 0 means success, there's no error. It's just a log by Bun, probably because of the use of --filter to target the website worspace. A "meta-log" from the task runner, essentially.
Also, there's an error at the end of the website build, but since the site appears to build correctly, I'm unsure what it means
Code 0 means success, there's no error. It's just a log by Bun, probably because of the use of
--filterto target the website worspace. A "meta-log" from the task runner, essentially.
Yes, it was just strange that it was reported as an error:
Right, I guess it is being outputed to stderr. Weird thing is I don't see it locally. Well, if it works, I guess it's fine.
Re:override, can you think of a small change I can make in the source to produce a test error for the wp package? The goal is to have a way to verify that it's being run with the local version and not one from npm.
I have a theory that it might not be necessary at all, because it should already be using the workspace: protocol and ~the npm version isn't even installed~ Nvm just realized it should be getting installed along with wp components. Either way, it'll be helpful to have a way to verify it.
Doesn't look like the Codesandbox CI supports Bun at all (inferred from what I've seen in the docs, there's no way to install it). Might be better to just remove it. The stackblitz alternative depends on your own CI so it should be much easier to use. Maybe I'll try to get that done in a separate PR before merging this one.
Huh, did you delete your last comment with the trick to make tests fail? @diegohaz
I reloaded the issue and it's gone now.
Good news, seems like the override is entirely unnecessary. I used your trick to test. I'm gonna wait for CI to pass and then push a commit with the trick to verify that CI behavior is the same. But I think we're in the clear with that.
Remaining:
- patch-package thing
- CodeSandbox (maybe we just remove it here?)
- test the untested stuff
Huh, did you delete your last comment with the trick to make tests fail? @diegohaz
I reloaded the issue and it's gone now.
Sorry, that information wasn't accurate.
If @wordpress/components relies on the same version as the local package, npm seems to automatically resolve it to the local package. The issue arises when there's a version mismatch. In that case, the remote npm package will be installed inside node_modules/@wordpress/components/node_modules.
This should make npm run test-chrome wordpress fail some tests. But I'm not sure how to manually trigger this while making overrides work simultaneously to verify if overrides are working.
If this is correct, it will become a problem if Ariakit 0.5.0 is released and @wordpress/components still depends on ^0.4.15.
Could it be replicated if I manually set the local version to 0.5 temporarily?
I think it's easier if you install a version of @wordpress/components that depend on ariakit@^0.3 instead like @wordpress/[email protected]. I couldn't reproduce it consistently, though.
FWIW, I tweaked the package.json and ran bun i:
The trick still causes it to fail. Not sure if that's enough to verify but it seems like a good sign.
I'll leave figuring this out to you if you don't mind. I'm not aware of all the constrains here.
This in the bun.lock file is a bit sus:
Here's how you can check if overrides is working using npm:
-
Install
@wordpress/[email protected](which depends on@ariakit/react@^0.3):npm i @wordpress/[email protected] -
Verify whether the nested dependency has been overridden:
test -e node_modules/@wordpress/components/node_modules/@ariakit/react/package.json && echo "duplicate dependency" || echo "hoisted dependency"If it outputs
hoisted dependency, the overrides are working.
You can remove the overrides option and repeat the steps. It should output duplicate dependency.
~Seems like patch-package works as it is. I've also read that the Bun patching implementation is a bit buggy still so we can just migrate in the future.~
Update: migrated anyway, seems to work fine.
Blocked by https://github.com/oven-sh/bun/issues/17192