fix(pages-router): keep 'use server' files outside app/ as normal modules (#1476)#1679
fix(pages-router): keep 'use server' files outside app/ as normal modules (#1476)#1679james-elicx wants to merge 1 commit into
Conversation
…ules (#1476) Server Actions are an App Router-only feature. When a Pages Router page imported a file containing the `"use server"` directive, `@vitejs/plugin-rsc`'s `rsc:use-server` transform rewrote the exports into server-reference proxies. Calling such a function from `getServerSideProps` then threw "Server Functions cannot be called during initial render." Add a pre-transform that strips the `"use server"` directive prologue from any file outside the `app/` directory before plugin-rsc sees it, mirroring Next.js's SWC pipeline (the server-actions pass only runs for the App Router layer). Files inside `app/` are unaffected, so existing server-action behaviour is preserved. Regression test ported from Next.js `test/e2e/app-dir/action-in-pages-router/`.
|
/bigbonk review |
commit: |
|
There was a problem hiding this comment.
Clean fix — the approach is correct, the implementation is careful, and the test coverage is sufficient for the stated problem.
What works well:
-
Correct scoping. Stripping the file-level directive prologue (not inline
"use server"inside function bodies) is exactly right — inline directives inside function bodies within pages code are already harmless because they don't trigger@vitejs/plugin-rsc's module-level transform gate. -
Proper plugin ordering.
enforce: "pre"ensures this runs before the RSC plugin'srsc:use-servertransform. The cheapcode.includes("use server")pre-check avoids cost on the vast majority of files. -
Robust directive parsing. The
stripUseServerDirectivefunction correctly handles BOM, hashbang, comments, whitespace, and other directives preceding"use server"(e.g."use strict"). Follows ECMA-262 Directive Prologue grammar faithfully. -
Timing safety.
appDiris assigned inconfig()→ the strip plugin reads it inconfigResolved()→ Vite guarantees allconfig()hooks complete before anyconfigResolved()fires. The closure-basedgetAppDir/hasAppDircallbacks are the same pattern used bycreateMiddlewareServerOnlyPlugin. -
Good test. The fixture mirrors the Next.js test (
test/e2e/app-dir/action-in-pages-router/) well, and the test assertion checks both the presence of the test element and the real return value ("action:foo"), which would fail if the export were still a server-reference proxy.
One thing to be aware of (not blocking):
There's an inherent trade-off in the path-based scoping: if a file with "use server" lives in a shared directory (e.g. lib/) and is imported by both a pages route and an app route, the directive gets stripped for both importers since Vite's module graph is shared. The app route consumer silently loses the server action. This matches Next.js behavior (SWC's isAppRouterPagesLayer gate is per-compilation-layer, and Next.js has separate bundler pipelines for pages vs app), but is worth noting for anyone who later tries to share "use server" files across routers. Not actionable now — just documenting the design constraint.
LGTM.
|
Review posted and approved on PR #1679. Summary: This is a clean, well-scoped fix. The plugin strips file-level |
Summary
"use server",@vitejs/plugin-rsc'srsc:use-servertransform rewrites the exports into server-reference proxies. Calling such a function fromgetServerSidePropsthen throws "Server Functions cannot be called during initial render."vinext:strip-non-app-use-server(anenforce: "pre"transform) that strips the"use server"directive prologue from any file outside theapp/directory before plugin-rsc sees it. Files insideapp/are unaffected, so existing server-action behaviour is preserved.packages/next/src/build/swc/options.tsisAppRouterPagesLayer).Fixes #1476.
Test plan
tests/app-router.test.tsreproduces the failure onmainand passes with the fix:pnpm test tests/app-router.test.ts -t "use server"tests/app-router.test.tssuite still green (333 tests, including the existing server-action coverage).tests/pages-router.test.tssuite still green (265 tests).tests/form.test.ts,tests/app-rsc-handler.test.ts,tests/app-server-action-execution.test.tsstill green.pnpm run check(format, lint, types) green.Ported from Next.js:
test/e2e/app-dir/action-in-pages-router/action-in-pages-router.test.ts.