chore: remove legacy Express entry and supporting files#82
Conversation
Now that npm run dev uses the Hono dev server (server/dev.ts), the Express-based entry point and its 2,226 lines of supporting code are no longer needed. Removed: server/index.ts, server/routes.ts, server/storage.ts, server/vite.ts Removed: dev:legacy script from package.json Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review Please evaluate:
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe pull request removes the entire backend Express server infrastructure, including the server entrypoint, HTTP routing module, storage initialization, Vite development utilities, and the associated npm script. Total deletion spans five files containing backend request handling, middleware, route registration, and development server setup logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR Review: Remove Legacy Express Entry and Supporting Files This is a clean, well-scoped deletion PR completing the Express to Hono migration. Overall looks good to merge. A few observations: WHAT IS RIGHT Aligns with documented roadmap: CLAUDE.md explicitly calls out these four files as legacy Express code kept for standalone dev reference and Phase 1.5 marked the Hono route migration as COMPLETED. This PR is the logical follow-through. routes.ts was a liability: The file had @ts-nocheck at the top and contained direct Drizzle queries in route handlers (e.g. db.select().from(schema.properties)), which violates the storage abstraction rule documented in CLAUDE.md. Removing 2,056 lines of type-unsound, pattern-violating code is unambiguously good. storage.ts shim was genuinely dead weight: The 9-line file with @ts-nocheck just re-exported from ./storage/system and ./storage/standalone. If nothing imports ./storage anymore (which the PR claims), this is clean. 212 tests passing across 26 files is solid signal that nothing live was depending on these files. ONE THING TO VERIFY BEFORE MERGE The test plan has one unchecked item: Workers Build deploys. Given this removes the Vite middleware shim (server/vite.ts) and Express entry (server/index.ts), the Workers build itself should not be affected since it targets server/worker.ts, not server/index.ts. But if there is a CI run that validates the Workers build, worth confirming it passes before the auto-merge squash fires. MINOR OBSERVATIONS (non-blocking) server/db.ts is not removed here — consistent with CLAUDE.md noting it separately as standalone dev reference, intentionally out of scope. Worth a follow-up cleanup ticket if/when standalone mode is retired. The index.ts had an indentation anomaly on lines 63-64 (PORT declaration outdented vs the rest of the IIFE). Not a concern since it is deleted, but confirms the file was accumulating drift. No security surface changes — purely removing dead code paths. VERDICT: Approve once the Workers build check is confirmed green. Nothing in this diff introduces risk; it only removes it. |
There was a problem hiding this comment.
Pull request overview
Removes the legacy Express development entrypoint and its supporting modules so local development runs exclusively via the Hono dev server (server/dev.ts), completing the migration started in PR #81.
Changes:
- Deleted the legacy Express server entrypoint and route/middleware helpers (
server/index.ts,server/routes.ts,server/vite.ts,server/storage.ts). - Removed the
dev:legacynpm script sonpm run devis the only dev entrypoint.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/index.ts | Removed the legacy Express server entrypoint. |
| server/routes.ts | Removed the monolithic legacy Express routes implementation. |
| server/storage.ts | Removed the legacy Express-mode storage shim. |
| server/vite.ts | Removed Express-coupled Vite dev/static serving helpers. |
| package.json | Removed dev:legacy script; dev now points only to server/dev.ts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -5,7 +5,6 @@ | |||
| "license": "MIT", | |||
| "scripts": { | |||
| "dev": "NODE_ENV=development tsx server/dev.ts", | |||
There was a problem hiding this comment.
npm run dev no longer starts the legacy Express server (and no longer serves the Vite client). There are still repo docs that describe npm run dev as “Express API + React/Vite client on :5000” and reference server/index.ts/server/routes.ts (e.g., README.md and CLAUDE.md). Please update those docs to reflect the Hono-only dev workflow (and how to run the Vite dev server/proxy) so new contributors don’t follow broken instructions.
| "dev": "NODE_ENV=development tsx server/dev.ts", | |
| "dev": "NODE_ENV=development tsx server/dev.ts", | |
| "dev:api": "NODE_ENV=development tsx server/dev.ts", | |
| "dev:client": "vite", | |
| "dev:client:host": "vite --host", |
Summary
npm run devnow exclusively uses the Hono dev server (server/dev.ts)dev:legacyscript — no longer neededRemoved
server/index.ts— Express entry point (72 lines)server/routes.ts— Express API routes (2,056 lines, duplicated by Hono route modules)server/storage.ts— Legacy storage shim (9 lines)server/vite.ts— Express Vite middleware (89 lines)Context
PR #81 migrated
npm run devto Hono and removed 15 legacy lib modules. This PR completes the migration by removing the Express entry itself.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor