Skip to content

Commit

Permalink
Node/Yarn to Bun (#624)
Browse files Browse the repository at this point in the history
Converts repo from using Node and Yarn to just using Bun, and upgrades
packages (notably Vite -> v5).

This is not absolutely necessary. In my testing, it saves some headache
and time, but it's also possible it could cause a problem down the road.
See the added note in the readme.

I think using Bun just as a package manager is very low risk, so at the
very least we could replace Yarn with Bun. In my testing, starting from
cleared caches, Yarn took 1 minute to install the frontend dependencies,
and Bun took 6 seconds (where simply downloading the packages was the
main bottleneck). This could save us time waiting for frontend tests to
finish, and possibly some $$ if we were to ever go over GitHub Actions
quota.

Using Bun as a Node replacement, again from anecdotal evidence, is also
fairly low risk. They seem to have already covered the general, commonly
used Node functionality, and have tested popular tools like Vite to make
sure they work with Bun. In some edge cases though, Bun might not behave
_exactly_ as Node. But that is a bug in Bun, as they explicitly say that
anything in Node should also work in Bun. The other reason it's low risk
is that it shouldn't be difficult to go back to Node if there is ever a
problem. The biggest risk, I think, is there being a problem, and the
maintainers not being aware that it could be a Bun bug.

Using Bun as a bundler or test runner would probably be a bigger change,
and could cause more problems, in my estimation, so that is left out of
this PR.

Another caveat: Bun's support of Windows is still in progress. They say
about 90% of Bun's test suite passes on Windows.

@kevinschaper If someone has a laptop that does NOT have node or yarn
installed on it, I'd be interested to test to make sure that _just_
installing Bun will work as expected. Theoretically, Bun should be the
only installation requirement for the frontend after this.

---------

Co-authored-by: Glass <[email protected]>
  • Loading branch information
vincerubinetti and glass-ships authored Mar 11, 2024
1 parent 8c541b7 commit 5c1d4a2
Show file tree
Hide file tree
Showing 19 changed files with 169 additions and 4,495 deletions.
10 changes: 4 additions & 6 deletions .github/workflows/build-and-deploy-images.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,14 @@ jobs:
with:
fetch-depth: 0 # to get all tags

- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: "18"
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: yarn install
run: bun install

- name: Build app
run: yarn build
run: bun run build

- name: Generate Image Tag for Frontend Dir
id: get-tag
Expand Down
99 changes: 33 additions & 66 deletions .github/workflows/test-frontend.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,130 +12,97 @@ defaults:
working-directory: ./frontend

jobs:
# build cache for rest of jobs
install-cache:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Node
uses: actions/setup-node@v4
with:
node-version: "18"

- name: Install packages
run: yarn install

- name: Install Playwright
run: npx playwright install

- name: Define cache key
id: define-key
run: echo "key=${{ hashFiles('frontend/yarn.lock') }}" >> $GITHUB_OUTPUT

- name: Store cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ steps.define-key.outputs.key }}

outputs:
cache-key: ${{ steps.define-key.outputs.key }}

# test that app can build without issues
test-build:
runs-on: ubuntu-latest
needs: install-cache
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ needs.install-cache.outputs.cache-key }}
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: bun install

- if: runner.debug == '1'
uses: mxschmitt/action-tmate@v3

- name: Run test
run: yarn build
run: bun run build

# test that app has no typescript errors
test-types:
runs-on: ubuntu-latest
needs: install-cache
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ needs.install-cache.outputs.cache-key }}
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: bun install

- if: runner.debug == '1'
uses: mxschmitt/action-tmate@v3

- name: Run test
run: yarn test:types
run: bun run test:types

# test that app is properly formatted and linted
test-lint:
runs-on: ubuntu-latest
needs: install-cache
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ needs.install-cache.outputs.cache-key }}
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: bun install

- if: runner.debug == '1'
uses: mxschmitt/action-tmate@v3

- name: Run test
run: yarn test:lint
run: bun run test:lint

# run unit tests
test-unit:
runs-on: ubuntu-latest
needs: install-cache
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ needs.install-cache.outputs.cache-key }}
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: bun install

- name: Run test
run: yarn test:unit
run: bun run test:unit

# run end to end integration tests
test-e2e:
runs-on: ubuntu-latest
needs: install-cache
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore cache
uses: actions/cache@v4
with:
path: ${{ env.CACHE_PATH }}
key: ${{ needs.install-cache.outputs.cache-key }}
- name: Set up Bun
uses: oven-sh/setup-bun@v1

- name: Install packages
run: bun install

- name: Install Playwright
run: bunx playwright install

- if: runner.debug == '1'
uses: mxschmitt/action-tmate@v3

- name: Run test
run: yarn test:e2e
run: bun run test:e2e
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ install-backend:
.PHONY: install-frontend
install-frontend:
cd frontend && \
yarn install && \
npx playwright install
bun install && \
bunx playwright install


.PHONY: model
Expand Down Expand Up @@ -134,15 +134,15 @@ test-backend:
.PHONY: test-frontend
test-frontend:
cd frontend && \
yarn test
bun run test


### Development ###

.PHONY: dev-frontend
dev-frontend: frontend/src/api/model.ts
cd frontend && \
VITE_API=local yarn dev
VITE_API=local bun run dev


.PHONY: dev-api
Expand Down Expand Up @@ -190,7 +190,7 @@ lint: lint-frontend lint-backend
.PHONY: lint-frontend
lint-frontend:
cd frontend && \
yarn test:lint
bun run test:lint


.PHONY: lint-backend
Expand All @@ -212,4 +212,4 @@ format-backend:
.PHONY: format-frontend
format-frontend:
cd frontend && \
yarn lint
bun run lint
35 changes: 35 additions & 0 deletions frontend/.eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"root": true,
"extends": [
"plugin:vue/vue3-recommended",
"plugin:vuejs-accessibility/recommended",
"eslint:recommended",
"@vue/eslint-config-typescript",
"@vue/eslint-config-prettier/skip-formatting",
],
"parserOptions": {
"ecmaVersion": "latest",
},
"rules": {
"prettier/prettier": "warn",
"vuejs-accessibility/anchor-has-content": [
"error",
{
"accessibleDirectives": ["tooltip"],
},
],
"vuejs-accessibility/label-has-for": [
"error",
{
"controlComponents": ["AppInput"],
"required": {
"some": ["nesting", "id"],
},
"allowChildren": true,
},
],
"vue/no-v-html": ["off"],
"vue/no-v-text-v-html-on-component": ["off"],
"vuejs-accessibility/mouse-events-have-key-events": ["off"],
},
}
54 changes: 0 additions & 54 deletions frontend/.eslintrc.js

This file was deleted.

Loading

0 comments on commit 5c1d4a2

Please sign in to comment.