chore: ビルドツールチェーンをVite+に移行#107
Conversation
tsup/vitest/biomeをvite-plusに統合し、設定をvite.config.tsに一元化する。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| registry-url: https://registry.npmjs.org/ | ||
| - run: npm ci | ||
| - run: npm run build | ||
| - uses: voidzero-dev/setup-vp@v1 |
There was a problem hiding this comment.
🔴 Third-party GitHub Action voidzero-dev/setup-vp is not pinned to a commit hash
All other third-party GitHub Actions in the repo are pinned to full commit hashes (e.g. actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd, actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f), but the new voidzero-dev/setup-vp@v1 uses only a mutable version tag. This is a supply-chain security risk: a compromised upstream repo could push malicious code to the v1 tag. This is especially dangerous in publish.yml where the job has id-token: write permissions and access to NPM_TOKEN, meaning an attacker could steal the token or inject malicious code into the published package.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -16,4 +13,23 @@ export default defineConfig({ | |||
| ), | |||
There was a problem hiding this comment.
🚩 Path alias resolution relies on vite-plus handling resolve.alias during pack
Previously, the build pipeline used tsup && tsc-alias -p tsconfig.json --outDir dist where tsc-alias explicitly resolved TypeScript path aliases (#domain/*, #application/*, #infrastructure/*) in the compiled output. Now, the build relies on vp pack using the resolve.alias configuration in vite.config.ts:6-13. If vp pack (which uses vite-plus-core internally) doesn't resolve these aliases in the bundled output, the published package would contain unresolved #domain/... imports, breaking consumers. This should be verified by inspecting the build output (dist/) to ensure aliases are properly resolved.
(Refers to lines 6-13)
Was this helpful? React with 👍 or 👎 to provide feedback.
| pack: { | ||
| entry: ["src/index.ts"], | ||
| format: ["esm", "cjs"], | ||
| dts: true, | ||
| sourcemap: true, | ||
| }, |
There was a problem hiding this comment.
🚩 vp pack config lacks clean option that tsup had
The old tsup.config.ts had clean: true which removed the dist/ directory before each build. The new vite.config.ts pack config (vite.config.ts:26-31) doesn't include an equivalent clean option. This could potentially leave stale files in dist/ across builds if entry points or output file names change. In practice this is low-risk since CI always starts fresh, but local development builds could accumulate stale artifacts.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
tsup / Vitest / Biome を廃止して vite-plus(vp)に統合し、ビルド・テスト・lint/fmt の設定を vite.config.ts に集約、あわせて CI / publish ワークフローを vp ベースに更新する PR です。
Changes:
vite.config.tsに test/lint/fmt/pack 設定を集約し、defineConfigをvite-plusに切り替えpackage.jsonの scripts / devDependencies をvite-plus基準に刷新(tsup/vitest/biome/tsc-aliasを削除)- GitHub Actions を
vp check/vp test/vp pack実行へ更新、テストの import をvite-plus/testに移行
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.ts |
Vite+ 設定へ統合(alias/test/lint/fmt/pack) |
tsup.config.ts |
tsup 設定を削除(Vite+ pack へ移行) |
biome.json |
Biome 設定を削除(Vite+ lint/fmt へ移行) |
package.json |
scripts を vp へ更新、依存関係を vite-plus に集約 |
package-lock.json |
ツールチェーン置換に伴う lock 更新 |
tests/domain/shared/StudentId.test.ts |
テスト import を vite-plus/test に変更 |
tests/domain/base/ValueObject.test.ts |
同上 |
tests/domain/aggregates/member/Member.test.ts |
同上 |
tests/domain/aggregates/karte/WorkDuration.test.ts |
同上 |
tests/domain/aggregates/karte/Karte.test.ts |
同上 |
.github/workflows/ci.yml |
CI を vp check/test/pack に更新 |
.github/workflows/publish.yml |
publish 前のビルドを vp pack に更新 |
Comments suppressed due to low confidence (1)
vite.config.ts:28
biome.jsonpreviously enforced tab indentation and double quotes (and enabled import organizing / recommended lints). After moving fmt/lint config here, those style/rule settings are no longer expressed, so runningvp fmt/vp lintmay produce different formatting and lint behavior than before. Consider adding the equivalent formatting/lint rule settings in the Vite+ config (if supported) to preserve the repo’s established style and avoid large reformat diffs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "devDependencies": { | ||
| "@biomejs/biome": "1.9.4", | ||
| "@types/node": "^22.9.0", | ||
| "@types/pg": "^8.16.0", | ||
| "drizzle-kit": "^0.31.8", | ||
| "tsc-alias": "^1.8.16", | ||
| "tsup": "^8.5.1", | ||
| "typescript": "^5.6.3", | ||
| "vitest": "^4.0.18" | ||
| "vite-plus": "^0.1.13" | ||
| }, |
There was a problem hiding this comment.
vite-plus in the lockfile requires Node ^20.19.0 || >=22.12.0, but this repo doesn’t declare a Node engine range in package.json. Adding an engines.node constraint (and optionally engineStrict guidance) would make the supported Node versions explicit for contributors and prevent installs failing on older Node 20/22 minors.
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm run lint:ci | ||
|
|
||
| typecheck: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm run typecheck | ||
| - uses: voidzero-dev/setup-vp@v1 | ||
| - run: vp check |
There was a problem hiding this comment.
voidzero-dev/setup-vp@v1 is not pinned to a commit SHA. This repo pins GitHub Actions by SHA elsewhere (e.g., actions/checkout), and leaving this unpinned risks supply-chain attacks or unexpected behavior changes when the tag moves. Pin this action to a specific commit SHA (and keep the version comment) to match the repo’s security posture.
| - uses: voidzero-dev/setup-vp@v1 | ||
| - run: vp check | ||
|
|
There was a problem hiding this comment.
CI runs vp check/test/pack as a bare command, which relies on setup-vp behavior and can drift from the project’s pinned vite-plus version in package-lock.json. To ensure CI always uses the repo-installed toolchain, prefer invoking via npm run check / npm test / npm run build (or npx vp ...) after npm ci.
| registry-url: https://registry.npmjs.org/ | ||
| - run: npm ci | ||
| - run: npm run build | ||
| - uses: voidzero-dev/setup-vp@v1 |
There was a problem hiding this comment.
voidzero-dev/setup-vp@v1 is not pinned to a commit SHA. For supply-chain safety and consistency with the rest of this repo’s workflows (which pin actions), pin this action to a specific SHA instead of a moving tag.
| - uses: voidzero-dev/setup-vp@v1 | |
| - uses: voidzero-dev/setup-vp@<COMMIT-SHA-FOR-v1> # pinned from v1 |
| - run: npm ci | ||
| - run: npm run build | ||
| - uses: voidzero-dev/setup-vp@v1 | ||
| - run: vp pack |
There was a problem hiding this comment.
Like CI, this workflow runs vp pack directly, which can end up using a globally installed vp from setup-vp rather than the repo’s locked vite-plus version. Prefer npm run build (or npx vp pack) so the publish artifact is built with the exact dependency version installed by npm ci.
| - run: vp pack | |
| - run: npx vp pack |
vite-plusが要求するesbuild@^0.27.0がlockfileで正しく解決されていなかった。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vite-plusとdrizzle-kitが異なるesbuildバージョンに依存しており、 ネスト解決時にoptionalフラグが消失してCIのnpm ciが失敗していた。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
biomeからoxfmtへの移行に伴い、フォーマッタの差異を解消する。 タブインデントは維持。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
npmがpackage.jsonのフォーマットを書き戻すため、oxfmtとの間で 永続的な差異が発生する。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
npm ciがpackage.jsonのフォーマットと非互換なため、 setup-nodeアクションにsetup-vpを統合しvp installを使用する。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
📝 Info: DrizzleMemberRepository indentation normalized from spaces to tabs
This file was previously indented with spaces while the rest of the codebase used tabs. The new formatter (oxfmt) normalized it to tabs, consistent with the useTabs: true setting in vite.config.ts:24. The logic is completely unchanged — verified line-by-line that this is purely a whitespace transformation.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: All TypeScript source changes are purely formatting — no logic changes
I verified every changed .ts file under src/ and tests/. All changes are whitespace/line-wrapping differences caused by the new formatter (wider print width than biome's default). No function signatures, logic, control flow, or argument ordering was modified. For example, the constructor argument order in src/executable/event.ts for RegisterMemberToExhibit(eventRepo, memberRepo) and RemoveMemberFromExhibit(memberRepo, eventRepo) was verified to match their class definitions unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
| cache: npm | ||
| - run: npm ci | ||
| - uses: voidzero-dev/setup-vp@v1 | ||
| - run: vp install |
There was a problem hiding this comment.
🚩 npm cache setting may be ineffective with vp install
The setup-node action at .github/actions/setup-node/action.yml:10 still uses cache: npm in the actions/setup-node step, but dependencies are now installed via vp install (line 12). If vp install doesn't use npm's global cache directory, this cache configuration is a no-op. Not harmful, but the cache hit/miss ratio should be monitored to ensure CI isn't slower than expected.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "build": "vp pack", | ||
| "prepare": "npm run build", |
There was a problem hiding this comment.
🚩 prepare script relies on vp binary availability during npm install
The prepare script in package.json:31 runs npm run build → vp pack. This hook executes automatically after npm install. For developers, this requires vp to be available in PATH. If vite-plus npm package provides the vp binary via node_modules/.bin/, this works fine since npm scripts resolve binaries from there. However, if vp is a separate global CLI (as suggested by the CI using voidzero-dev/setup-vp@v1 as a distinct setup step), then local npm install would fail at the prepare step for developers who haven't globally installed vp. The README at README.md:69 still instructs npm install for developer setup without mentioning vp installation.
Was this helpful? React with 👍 or 👎 to provide feedback.
npmがpackage.jsonを独自フォーマットで書き戻すため、 フォーマッタとの間で永続的な差異が発生する。 これはnpm側が意図的に対応しない既知の問題。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tsgolintがbaseUrlを非推奨として拒否するため削除。 pathsの解決はbaseUrl無しでも動作する。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
oxlintのtype-awareルールに対応し、IIFEに.catchを追加。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
catchパラメータにunknown型を明示し、process参照を削除。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| node-version: "22.x" | ||
| cache: npm | ||
| - run: npm ci | ||
| - uses: voidzero-dev/setup-vp@v1 |
There was a problem hiding this comment.
🚩 GitHub Action voidzero-dev/setup-vp@v1 is not SHA-pinned
All other GitHub Actions in the workflows (actions/checkout, actions/setup-node) are pinned to specific commit SHAs for supply-chain security. The newly added voidzero-dev/setup-vp@v1 at .github/actions/setup-node/action.yml:11 and .github/workflows/publish.yml:28 is only pinned to a mutable tag (v1). This is inconsistent with the existing security posture and could allow a compromised or updated action to run in CI without notice.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "paths": { | ||
| "#domain": ["./src/domain"], | ||
| "#domain/*": ["./src/domain/*"], |
There was a problem hiding this comment.
📝 Info: Removal of baseUrl from tsconfig.json is safe with TypeScript 5.x
The baseUrl field was removed from both tsconfig.json and example/tsconfig.json. In TypeScript 5.0+, paths can be specified without baseUrl and are resolved relative to the tsconfig.json file location. Since this project uses typescript: ^5.6.3, this is correct. The example tsconfig compensated by changing the path from ["./dist"] (resolved relative to old baseUrl: "..") to ["../dist"] (resolved relative to the tsconfig.json itself), which is equivalent.
(Refers to lines 10-17)
Was this helpful? React with 👍 or 👎 to provide feedback.
| check: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm run lint:ci | ||
|
|
||
| typecheck: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm run typecheck | ||
| - run: vp check |
There was a problem hiding this comment.
📝 Info: CI merged separate lint and typecheck jobs into single check job
The old CI had separate lint (running biome ci .) and typecheck (running tsc --noEmit) jobs that ran in parallel. The new CI at .github/workflows/ci.yml:22-28 combines them into a single check job running vp check. This changes the CI feedback model — previously a lint failure and a type error would surface independently in parallel; now they are sequential in one job. Not a bug, but a behavioral change that could slow down feedback if one check is slow.
Was this helpful? React with 👍 or 👎 to provide feedback.
exampleは独自tsconfigを持つサンプルコードであり、 ルートtsconfigのtype-awareチェックと互換性がない。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm test | ||
|
|
||
| drizzle-check: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: ./.github/actions/setup-node | ||
| - run: npm run db:generate | ||
| - name: Check for uncommitted schema changes | ||
| run: | | ||
| if [ -n "$(git status --porcelain drizzle/)" ]; then | ||
| echo "::error::Drizzle schema and generated files are out of sync. Run 'npm run db:generate' and commit the result." | ||
| git add -N drizzle/ | ||
| git diff drizzle/ | ||
| exit 1 | ||
| fi | ||
| - run: vp test | ||
|
|
There was a problem hiding this comment.
🚩 Drizzle schema drift check removed from CI
The drizzle-check job was removed from ci.yml. This job previously ran npm run db:generate and then checked git status --porcelain drizzle/ to ensure generated Drizzle schema files were in sync with the source definitions. Without this check, developers could accidentally commit source schema changes without regenerating the migration files, and CI would not catch it. It's unclear whether vp check includes an equivalent validation. If it doesn't, this is a CI coverage regression.
(Refers to lines 22-37)
Was this helpful? React with 👍 or 👎 to provide feedback.
## Why PR #107(VP移行)のマージでdrizzle-checkジョブが欠落し、ブランチ保護の必須チェックが永遠にpendingになっていた。 ## What ci.ymlにdrizzle-checkジョブを復元。 ## Test plan - [x] CI通過を確認 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/su-its/core/pull/121" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
vite.config.tsに一元化vp check/vp test/vp packに更新vitest→vite-plus/testに変更Why
開発ツールチェーンをVite+に統一することで、設定ファイルの分散を解消し、ビルド・リント・テストを単一のツールで管理する。
What
tsup.config.tsvite.config.ts(pack設定)vitest.config.tsvite.config.ts(test設定)biome.jsonvite.config.ts(lint/fmt設定)tsup,vitest,@biomejs/biome,tsc-aliasvite-plusTest plan
vp packでESM/CJS/DTSビルド成功vp testで全64テストパス#domain等)がdist出力で正しく解決されているvp check,vp test,vp pack)が通ること🤖 Generated with Claude Code