Fix Vite dev-server integration bugs#1746
Conversation
📝 WalkthroughWalkthroughThis PR enhances the Maizzle dev server by fixing server lifecycle management, improving configuration change handling with proper renderer state synchronization, clarifying configuration documentation, and adding comprehensive test coverage for dev server behaviors including shutdown and hot module reload events. ChangesDev Server Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vitest.config.ts (1)
9-19: ⚡ Quick winClarify the coverage exclusion rationale.
The comment states these surfaces are "exercised by the running dev server, not unit tests," but the review stack context indicates this PR adds "comprehensive Vitest suite validates dev server lifecycle" in
serve.test.ts. Consider revising the comment to acknowledge that while tests exist, coverage is excluded to keep metrics focused on the core library, or to reflect that integration-heavy code is better validated through running the dev server than coverage metrics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vitest.config.ts` around lines 9 - 19, Update the explanatory comment above the coverage.exclude entries in vitest.config.ts (the coverage: { exclude: [...] } block) to acknowledge that while there are tests (e.g., the comprehensive Vitest suite like serve.test.ts) that exercise dev-server lifecycle, these paths ('src/serve.ts' and 'src/server/**') are intentionally excluded so coverage metrics remain focused on the core library and because integration-heavy server lifecycle code is validated by running the dev server rather than line-level coverage; revise the comment to clearly state that rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@vitest.config.ts`:
- Around line 9-19: Update the explanatory comment above the coverage.exclude
entries in vitest.config.ts (the coverage: { exclude: [...] } block) to
acknowledge that while there are tests (e.g., the comprehensive Vitest suite
like serve.test.ts) that exercise dev-server lifecycle, these paths
('src/serve.ts' and 'src/server/**') are intentionally excluded so coverage
metrics remain focused on the core library and because integration-heavy server
lifecycle code is validated by running the dev server rather than line-level
coverage; revise the comment to clearly state that rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a1bd6d62-c4ad-46cf-bc88-d88a72341857
📒 Files selected for processing (6)
.gitignoresrc/plugin.tssrc/serve.tssrc/tests/serve.test.tssrc/types/config.tsvitest.config.ts
Summary by CodeRabbit
Bug Fixes
Documentation
vite.configfiles.Tests