fix: move optional dependencies to peerDependencies

[hyoban]

We are currently installing these dependencies on the user side.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@384

commit: 7f0592c

[james-elicx]

/bigbonk review

[ask-bonk]

Thanks for the PR — the intent makes sense. Moving @vitejs/plugin-rsc and react-server-dom-webpack out of dependencies prevents them from being unconditionally installed in users' node_modules, which is the right thing to do since they're only needed for App Router.

A few points to consider:

1. @vitejs/plugin-rsc and react-server-dom-webpack should arguably NOT be optional

These packages are required for any App Router project. vinext's index.ts throws a hard error if @vitejs/plugin-rsc isn't installed when app/ is detected (line 690). Similarly, the App Router entry files have static imports from @vitejs/plugin-rsc/rsc, @vitejs/plugin-rsc/ssr, and @vitejs/plugin-rsc/browser.

Marking them as optional peer deps means package managers won't warn users if they're missing. That pushes the error discovery to runtime rather than install time. The current code handles this gracefully with helpful error messages, so it's not catastrophic — but it's worth considering whether the "optional" annotation is the right signal.

For Pages Router-only projects, these truly aren't needed. So the "optional" marking is technically correct, but the ergonomics are a tradeoff.

2. Version range mismatch for react-server-dom-webpack

See inline comment — devDependencies pins ^19.2.4 but peerDependencies says >=19.2.0. These should be consistent.

3. @mdx-js/rollup was never a dependency — adding it as optional peer is good

This is a nice addition. The codebase already tries a dynamic import("@mdx-js/rollup") with a fallback warning, so declaring it as an optional peer dep makes the relationship explicit.

Overall this is a reasonable change — the core idea of not force-installing these on every user is correct. Just the version range inconsistency should be addressed.

[ask-bonk]

Given that @vitejs/plugin-rsc is required for App Router (vinext throws a hard error at index.ts:690 if it's missing when app/ is detected, and App Router entries have static imports from it), marking it as optional may cause confusion. Users who set up an App Router project and skip this dependency won't get a package manager warning — they'll only find out at runtime.

Same applies to react-server-dom-webpack — it's a transitive requirement of @vitejs/plugin-rsc.

This is arguably fine since vinext has good runtime error messages, and Pages Router users genuinely don't need these. Just flagging it as a design tradeoff worth an explicit decision.

[ask-bonk]

Review posted. Here's a summary of the feedback:

1. Version range mismatch — react-server-dom-webpack is ^19.2.4 in devDependencies but >=19.2.0 in peerDependencies. Suggested aligning to ^19.2.4 via a code suggestion.

2. Optional peer deps tradeoff — @vitejs/plugin-rsc and react-server-dom-webpack are required for App Router (vinext throws hard errors if missing). Marking them optional means package managers won't warn users at install time. Flagged this as a design decision worth an explicit call.

3. Positive note on @mdx-js/rollup — Adding it as an optional peer dep is a good change since the codebase already does dynamic imports with graceful fallbacks.

github run

[hyoban]

Version is updated for react-server-dom-webpack. Since these two dependencies are indeed optional for the pages router, I don't plan to change them.

[james-elicx]

/bigbonk re-review

[ask-bonk]

LGTM. This is a correct and well-scoped change.

What this does: Moves @vitejs/plugin-rsc and react-server-dom-webpack from dependencies (unconditionally installed) to optional peerDependencies (user manages installation). Adds @mdx-js/rollup as a new optional peer dep.

Why it's correct:

• @vitejs/plugin-rsc is only needed for App Router projects. The codebase already handles its absence gracefully — index.ts pre-resolves it via createRequire in a try/catch, and only throws a clear error if App Router is detected without it. Pages Router projects work fine without it.
• react-server-dom-webpack is never directly imported by vinext — it's consumed transitively through @vitejs/plugin-rsc's subpath exports. Declaring it as an optional peer makes the relationship explicit for App Router users without forcing installation on Pages Router users.
• @mdx-js/rollup already uses a dynamic import() with a try/catch fallback (index.ts:982), logging a warning if missing. Adding it as an optional peer documents the relationship without introducing a hard requirement.
• Version ranges now align: ^0.5.19 for @vitejs/plugin-rsc and ^19.2.4 for react-server-dom-webpack in both devDependencies and peerDependencies.
• The lockfile changes are minimal and expected — just moving entries between dependency categories.

The previous review's concern about optional peers masking install-time warnings is a reasonable tradeoff observation, but vinext's runtime error messages (init.ts:getInitDeps, deploy.ts missing-dep checks, and the startup error in index.ts) cover the user experience adequately.

[ask-bonk]

Review submitted. Here's a summary of my findings:

Approved — this is a correct and well-scoped change.

The PR moves @vitejs/plugin-rsc and react-server-dom-webpack from hard dependencies to optional peerDependencies, and adds @mdx-js/rollup as a new optional peer dep. This prevents these packages from being unconditionally installed in every user's node_modules.

Key validation points:

• @vitejs/plugin-rsc is pre-resolved via createRequire in a try/catch (index.ts), with clear error messages when App Router needs it
• react-server-dom-webpack is never directly imported by vinext — it's consumed through @vitejs/plugin-rsc's subpath exports
• @mdx-js/rollup already uses dynamic import with graceful fallback
• Version ranges are aligned between devDependencies and peerDependencies (the earlier mismatch was already fixed)
• Runtime error messages in init.ts, deploy.ts, and index.ts adequately cover the missing-dependency UX

github run