Replace rolldown-vite with standard Vite for Node 18+ compatibility#7
Replace rolldown-vite with standard Vite for Node 18+ compatibility#7
Conversation
Co-authored-by: tomiwa-a <[email protected]>
…erabilities Co-authored-by: tomiwa-a <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR updates the repo’s Node/Vite toolchain and dependency set to address prior review feedback and resolve security issues, notably by switching the website back to standard Vite and bumping @modelcontextprotocol/sdk.
Changes:
- Replace
rolldown-vitewithvite@^6.4.1forapps/websiteand remove the Vite override. - Add a root
engines.nodeconstraint and document the Node version requirement in the README. - Update
@modelcontextprotocol/sdkto^1.26.0and regeneratepackage-lock.jsonaccordingly.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds root Node engine constraint. |
| package-lock.json | Dependency tree refresh reflecting Vite swap + SDK bump. |
| apps/website/package.json | Switches from rolldown-vite to standard vite. |
| apps/mcp/package.json | Bumps @modelcontextprotocol/sdk version. |
| README.md | Documents Node version requirements for contributors/users. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "engines": { | ||
| "node": "^18.0.0 || ^20.0.0 || >=22.0.0" | ||
| }, |
There was a problem hiding this comment.
The declared Node.js engine range allows Node 18.0–18.14.0, but the updated dependency tree includes packages that require at least Node >=18.14.1 (e.g., @hono/node-server pulled in via @modelcontextprotocol/sdk). Please tighten the engines.node constraint to match the actual minimum supported Node version (and regenerate package-lock.json afterwards).
| ## Quick Start | ||
|
|
||
| **Requirements:** | ||
| - Node.js `^18.0.0 || ^20.0.0 || >=22.0.0` |
There was a problem hiding this comment.
README Node.js requirement should match the repo’s actual minimum supported Node version. With the updated dependencies requiring >=18.14.1, the current ^18.0.0 documentation is too permissive; please update it to the same engines.node constraint used in the root package.json.
| - Node.js `^18.0.0 || ^20.0.0 || >=22.0.0` | |
| - Node.js `>=18.14.1` |
| "@modelcontextprotocol/sdk": "^0.6.0", | ||
| "@modelcontextprotocol/sdk": "^1.26.0", | ||
| "dotenv": "^16.3.1", | ||
| "zod": "^3.22.4" |
There was a problem hiding this comment.
@modelcontextprotocol/sdk@^1.26.0 declares a non-optional peer dependency on zod with range ^3.25 || ^4.0, but this workspace currently allows zod@^3.22.4. Please bump the zod dependency range here to satisfy the SDK’s peer requirement (e.g., >=3.25) to avoid peer-dep install failures/warnings and potential runtime incompatibilities.
| "zod": "^3.22.4" | |
| "zod": "^3.25.0" |
Addresses review feedback on #5:
[email protected]requires Node^20.19.0 || >=22.12.0, breaking installs for contributors on Node 18 or 20.0-20.18.Changes
npm:[email protected]withvite@^6.4.1(supports Node^18.0.0 || ^20.0.0 || >=22.0.0)engines.nodefield documenting supported versions@modelcontextprotocol/sdkfrom^0.6.0to^1.26.0to fix ReDoS and DNS rebinding vulnerabilitiesImpact
Contributors can now install dependencies on any Node 18+ version. All security vulnerabilities resolved (
npm auditreports 0 issues).✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.