-
Notifications
You must be signed in to change notification settings - Fork 5
feat: migrate from Node.js/npm to Bun #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: migrate from Node.js/npm to Bun #149
Conversation
|
@MiltonTulli Thanks a lot for the PR, im in for a change on stack, but what would be the pros of changing to bun & vite? |
- Change dev server port back to 3030 (vite.config.ts, playwright.config.ts) - Add overrides for @noble/hashes and @noble/curves to fix version conflict - Update package.json scripts to call tools directly (no bun run prefix) - Update all docs to use npm run/start commands while keeping bun install - Update shell scripts to use npm run and correct port 3030
|
I'm testing this branch, and it seems to be working quite well. I'm in for this change too There are still some references to npm. I don't know what you, @AugustoL, discussed with @MiltonTulli, but if my comments are fix we should merge so I can continue with #146 and #118 |
| ```bash | ||
| # Run with npm script | ||
| # Run with bun script | ||
| npm run dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some references to npm still.
Should some commands still run with npm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i specifically left the commands to be run with npm, I think it is better since we use it as an alias command and later on the package.json we can define the entire command to be run, and in case it changes we change it only on one place
| "build": "vite build", | ||
| "build:production": "bash ./scripts/build-production.sh", | ||
| "build:staging": "bash ./scripts/build-staging.sh", | ||
| "build:development": "webpack --config webpack.config.js --mode development", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this command removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mistake, going to add it back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commited here 4bd5164
| // These are injected into localStorage by the test fixture (e2e/fixtures/test.ts) | ||
| webServer: { | ||
| command: "npm start", | ||
| command: "npm run start", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If tests e2e still use npm, should we keep the .nvmrc file?
| - name: Setup Node.js for deploy action | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep using Node LTS if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use this version cause is what the deploy to ipfs needs to run, it wont affect or require to have nodejs installed locally. This is the only place where nodejs is used, we can leave it as it is.
| 3. Install dependencies: | ||
| ```bash | ||
| npm install | ||
| bun install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update all this section to use bun instead of npm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It uses bun for install, which is compatible with npm install. And te rest of the contirbuting file doesnt mention nodejs.
MatiasOS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
Description
Migrate the project tooling from Node.js/npm + Webpack to Bun + Vite for faster builds, better developer experience, and modern tooling.
Related Issue
Type of Change
Changes Made
Core Migration
Files Changed
package.jsonpackageManager, update scripts, remove eslintConfig, restorebuild:developmentbunfig.tomlvite.config.tsindex.htmlvitest.config.tsplaywright.config.tsscripts/*.sh.github/workflows/*.ymlREADME.md,CLAUDE.md,CONTRIBUTING.mdRemoved
.nvmrc- No longer neededpackage-lock.json- Replaced bybun.lockwebpack.config.js- Replaced by ViteeslintConfigin package.json - Using BiomeScreenshots (if applicable)
N/A - Tooling change, no UI changes
Checklist
bun run format:fixandbun run lint:fixbun run typecheckwith no errorsbun run test:runAdditional Notes
Test Results
bun install- Installs dependencies correctlybun start- Dev server runs on port 3000bun run build- Production build succeedsbun run test:run- All tests passbun run typecheck- No TypeScript errorsbun run format && bun run lint- No issuesCI Status Note
The build-and-deploy (PR Preview) check is expected to fail until this PR is merged. This is because
deploy-pr-preview.ymlusespull_request_target, which executes the workflow from the base branch (main), not the PR branch. Once merged, future PRs will use the correct Bun configuration.Node.js Usage
The only remaining Node.js usage is in
.github/workflows/hash-deploy-build.ymlfor the custom Pinata deploy action, which requires Node.js runtime for GitHub Actions.