fix(transforms): strip server-only data hooks from the client artifact (depends on #2999) by mattboon · Pull Request #3002 · veryfront/veryfront-code

mattboon · 2026-07-21T18:03:03Z

Summary

The browser artifact is compiled from the same source file as the server one, so getServerData / getStaticData / getStaticPaths bodies shipped to the client along with everything they import. A page whose loader reached node:crypto therefore linked against the node-builtin noop polyfill and hydration died with does not provide an export named 'createHash' — for code that never runs in the browser at all.

esbuild cannot solve this for us: in transform mode (as opposed to bundle mode) it never drops an import, because it cannot prove the module is side-effect free. Confirmed directly:

treeShaking: true   -> import { hashOf } from "@/lib/uses-crypto";   # kept
treeShaking: false  -> import { hashOf } from "@/lib/uses-crypto";   # kept

So the browser pipeline gains a stage that empties the hooks' bodies and then removes the imports left unreferenced.

Why empty the bodies rather than delete the declarations: the binding stays valid for any surviving export clause, so typeof mod.getServerData === "function" still holds for the client router's data-fetching check. Deleting the declaration would break that.

Every step is conservative — anything that cannot be parsed with confidence is left exactly as it was, and side-effect-only imports (import "./x") are never removed.

Reproduction

Route: d-server-static.tsx
Test: src/transforms/pipeline/stages/browser-server-exports-strip.test.ts

Test evidence

15 new cases covering function declarations, var-assigned arrows, all three hooks, aliased/namespace/default/side-effect imports, and the "leave it alone" paths.

ok | 22 passed (194 steps) | 0 failed      # src/transforms/pipeline/

Wider suite: deno task test:unit → 2525 passed | 0 failed.

SSR evidence

/test/d-server-static still renders server-side (the hook is untouched on the SSR path), and the server-only import is gone from the client artifact:

$ curl -s .../pages/test/d-server-static.js | grep -E '^import|getServerData'
import { jsx, jsxs } from "https://esm.sh/react@19.2.4/jsx-runtime?…"
async function getServerData(_ctx) { throw new Error('server-only'); }
                                              # "../../lib/uses-crypto.js" no longer imported

$ curl -so/dev/null -w '%{http_code}' http://localhost:3010/test/d-server-static
200

Client evidence

PASS /test/d-server-static    # was: SyntaxError … does not provide an export named 'createHash'

Chain: this PR is part of a 13-PR chain fixing the bugs catalogued in
veryfront-router-testing.
Its base is the previous PR in the chain, so the diff shows only this fix.
The root of the chain is #2999 (fix/ssr-lazy-import-graceful-degrade) — merge #2999 first, then
rebase the chain onto main.

Regression gate: deno task test:unit → 2525 passed | 0 failed; deno task lint,
deno task fmt:check and deno task typecheck all clean. The reproducer's full 56-route
matrix (ROUTES.txt + sweep.sh) was re-run after every fix: 7 routes improved, 0 regressed.
A 46-route Chromium hydration sweep (client-sweep.mjs) backs the client-side claims.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d1a8665c5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-21T18:07:19Z

+    const stripped = stripFunctionDeclaration(result, name, inert) ??
+      stripVariableDeclaration(result, name, inert);


Only strip hooks that are actually exported

When a page has a client helper named getServerData, getStaticData, or getStaticPaths that is not exported, this still strips the first matching declaration because it never checks the module's export list. For example, function getServerData() { ... } called by the default component now throws during hydration even though it was not a Veryfront data hook. Please gate the replacement on an actual named export before modifying the declaration.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-21T18:07:19Z

+    if (isInert(inert, i)) continue;
+    const ch = code[i];
+    if (ch === "{") depth++;
+    else if (ch === "}") {


Skip regex literals before counting hook braces

When an exported hook body contains a regex literal with a brace, such as /\}/, the scanner does not mark regex literals inert, so these brace counters can treat the regex character as the end of the function body. The replacement then cuts the body early and can leave statements outside the stub, producing an invalid browser module instead of safely stripping the hook.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-21T18:07:19Z

+    if (bindings.some((binding) => referenced.has(binding))) continue;
+    removals.push({ start: imp.ss, end: imp.se });


Preserve side-effectful value imports

When a binding is only referenced from the stripped hook, this removes the whole import declaration. Static imports with bindings still execute module top-level code, so a page like import { load } from './client-init-and-data.ts' loses any client registration or polyfill in that module even though only load was used by getServerData. Because this pass has no side-effect information, it should not delete value imports unless it can prove or mark them as server-only.

Useful? React with 👍 / 👎.

kwakayama

Score: 72/100

Requesting changes. The goal is valid, but the new browser server-export stripping stage is too heuristic for a compiler transform that is meant to prevent server-only imports from leaking into the browser artifact.

Blocking concern:

src/transforms/pipeline/stages/browser-server-exports-strip.ts removes imports by regex-counting identifiers in the remaining code. That count includes strings and comments, so an import used only by a stripped hook can be retained if the binding name appears in inert text. In that case the server-only dependency chain can still ship to the browser and preserve the hydration/link failure this PR is meant to remove.

Please use an AST/scope-aware identifier walk, or at minimum exclude strings/comments/templates from the reference scan, and add regressions for comment-only and string-only references. I would also like coverage for same-name non-exported declarations and future hook forms, since this stage currently relies on hand-rolled source scanning.

mattboon · 2026-07-21T19:47:26Z

Addressed. The stage no longer scans source text at all: it now runs on the repo's CodeParser (Babel) AST contract, the same reason src/rendering/rsc/export-extractor.ts already uses it.

The hand-rolled scanner is gone (findInertRegions, matchBrace, the two strip*Declaration helpers, importedBindings). referencedIdentifiers() walks the parsed tree, collects only Identifier/JSXIdentifier nodes, skips ImportDeclaration subtrees, and pre-marks non-reference positions (non-computed member properties and object/class keys). Comments, string literals, template chunks and JSX text are not identifier nodes, so they cannot enter the reference set by construction.

Regressions added for exactly what you asked for:

binding referenced only in a line comment, and only in a block comment
binding referenced only in a string literal
binding referenced only in template literal text, plus the inverted case: a reference inside ${...} is real code and must keep the import
binding appearing only as JSX text
same-name non-exported declaration: a private getServerData alongside a genuinely exported hook stays untouched
declaration forms: export async function, export const ... = async function, a hook declared before a separate export { ... } clause, and export { other as getServerData }, which must not strip the local other

Two other findings from the same review round are fixed here as well, since they had the same root cause. An unexported declaration is never emptied, and an import that falls out of use is reduced to a side-effect import rather than deleted, because this pass has no side-effect information about the module it points at. Node built-ins are the exception, since in the browser they resolve to a noop polyfill. That matches what esbuild does with an external import whose bindings go unused.

One honest caveat: a JSX attribute name (<p hashOf="1" />) is a JSXIdentifier and counts as a reference, so it keeps an import alive. That is the safe direction (over-counting only ever retains an import, it never ships a server-only chain) and the file documents it.

Verification: stage suite 42 steps passing, deno check clean on the test file and on src/transforms/index.ts, deno fmt/deno lint clean.

kwakayama

Follow-up after fixes: approving. The implementation now uses parsed AST references instead of regex/comment/string matching, and inert text regressions are covered. Local verification: \running 1 test from ./src/transforms/pipeline/stages/browser-server-exports-strip.test.ts
browser-server-exports-strip ...
emptying server-only hooks ...
empties an exported async function declaration body ... ok (10ms)
empties a directly exported function declaration ... ok (1ms)
replaces an exported arrow initialiser ... ok (1ms)
handles all three hooks in one module ... ok (1ms)
leaves a module without server hooks untouched ... ok (0ms)
does not treat a same-named string as a declaration ... ok (1ms)
leaves a non-exported function of the same name alone ... ok (1ms)
leaves a local declaration that is only aliased to a hook name alone ... ok (0ms)
empties a hook declared before a separate export clause ... ok (1ms)
keeps client code that follows a regular expression containing braces ... ok (1ms)
keeps client code after a division that looks like a regular expression ... ok (1ms)
handles a template literal with braces and interpolation ... ok (1ms)
handles minified single-line input ... ok (1ms)
handles TSX with types and JSX ... ok (2ms)
leaves a module that does not parse unchanged ... ok (1ms)
emptying server-only hooks ... ok (24ms)
import bindings ...
reduces an unreferenced project import to a side-effect import ... ok (1ms)
removes an unreferenced node builtin import outright ... ok (0ms)
keeps an import that the client still references ... ok (1ms)
keeps an import when only one of its bindings is used ... ok (0ms)
keeps a bare side-effect import untouched ... ok (1ms)
keeps a default import the client renders with ... ok (0ms)
reduces a namespace import the client no longer uses ... ok (1ms)
does not count a matching property name as a reference ... ok (0ms)
counts a computed property access as a reference ... ok (1ms)
counts a JSX component as a reference ... ok (0ms)
import bindings ... ok (5ms)
inert text is not a reference ...
does not count a line comment mention ... ok (0ms)
does not count a block comment mention ... ok (1ms)
does not count a string literal mention ... ok (0ms)
does not count a template literal mention ... ok (1ms)
counts a template literal interpolation, which is real code ... ok (0ms)
does not count a JSX text node mention ... ok (0ms)
inert text is not a reference ... ok (4ms)
declaration forms ...
leaves a private same-named declaration alone beside a real hook ... ok (0ms)
leaves a local aliased to a hook name alone beside a real hook ... ok (0ms)
empties a hook declared as an exported function expression ... ok (0ms)
empties a hook declared as a directly exported async function ... ok (0ms)
declaration forms ... ok (1ms)
plugin ...
drops the server-only import chain from the client artifact ... ok (1ms)
does not run for the ssr target ... ok (0ms)
plugin ... ok (1ms)
browser-server-exports-strip ... ok (37ms)

ok | 1 passed (42 steps) | 0 failed (39ms). Score: 94/100. Next step: merge after base stack and checks are green.

kwakayama · 2026-07-21T20:31:52Z

Clean follow-up after the approval above:

Score: 94/100.

Verification:

deno test --allow-all src/transforms/pipeline/stages/browser-server-exports-strip.test.ts

Next step: merge after the base stack and refreshed checks are green.

The base branch was changed.

kwakayama

Follow-up approval after #3001 merged and the stack was rebased onto main.

Score: 94/100.

Rationale: implementation still uses parsed AST references instead of regex/comment/string matching; inert text regressions remain covered after the rebase.

Verification:

deno task audit
combined targeted regression suite: 21 tests, 334 steps, 0 failed

Next step: wait for refreshed GitHub checks, then merge when green.

The browser artifact is compiled from the same source file as the server one, so `getServerData`/`getStaticData`/`getStaticPaths` bodies shipped to the client along with everything they import. A page whose loader reached `node:crypto` therefore linked against the node-builtin noop polyfill and hydration died with "does not provide an export named 'createHash'". esbuild cannot fix this for us: in transform mode (as opposed to bundle mode) it never drops an import, because it cannot prove the module is side-effect free. So the browser pipeline gains a stage that empties the hooks' bodies, keeping their bindings valid for any surviving export clause, and then drops the import bindings they were the last user of. The stage works on the `CodeParser` AST, for the reason `rendering/rsc/export-extractor.ts` already does, and as the equivalent passes in Next.js (SWC) and React Router (Babel) do: text matching cannot tell an exported hook from a private function of the same name, and counts braces inside regular expression literals. An import that falls out of use is reduced to a side-effect import rather than deleted, because this pass has no side-effect information about the module it points at. Node built-ins are the exception, since in the browser they resolve to a noop polyfill with nothing to preserve. This matches what esbuild does with an external import whose bindings go unused. Fixes bug 2 of the reproducer matrix.

kwakayama

Follow-up approval after rebasing #3002 onto the latest main.

Score: 94/100.

Rationale: the parser-backed transform and inert-text regressions are unchanged by the rebase, and the stack validated locally on the rebased head.

Verification:

deno task audit
combined targeted regression suite: 21 tests, 334 steps, 0 failed

Next step: wait for refreshed GitHub checks and the required reviewer gate, then auto-merge.

mattboon requested review from kojiwakayama and kwakayama as code owners July 21, 2026 18:03

mattboon mentioned this pull request Jul 21, 2026

fix(transforms): make leaked node: named imports fail at the call site (depends on #2999) #3004

Merged

chatgpt-codex-connector Bot reviewed Jul 21, 2026

View reviewed changes

mattboon force-pushed the fix/ts-extension-browser-mime branch from 28484d5 to 49b70d8 Compare July 21, 2026 18:14

mattboon force-pushed the fix/strip-server-exports-from-client-bundle branch from 4d1a866 to aea6a67 Compare July 21, 2026 18:15

kwakayama requested changes Jul 21, 2026

View reviewed changes

mattboon force-pushed the fix/strip-server-exports-from-client-bundle branch from aea6a67 to 01fe41d Compare July 21, 2026 19:22

mattboon force-pushed the fix/ts-extension-browser-mime branch from 49b70d8 to d2f9260 Compare July 21, 2026 19:44

mattboon force-pushed the fix/strip-server-exports-from-client-bundle branch from 01fe41d to 7c2f401 Compare July 21, 2026 19:44

kwakayama force-pushed the fix/ts-extension-browser-mime branch from d2f9260 to 2b6e6ce Compare July 21, 2026 20:30

kwakayama force-pushed the fix/strip-server-exports-from-client-bundle branch from 7c2f401 to 5b7f8ed Compare July 21, 2026 20:30

kwakayama previously approved these changes Jul 21, 2026

View reviewed changes

kwakayama force-pushed the fix/strip-server-exports-from-client-bundle branch from 5b7f8ed to 4fedb43 Compare July 21, 2026 20:38

kwakayama force-pushed the fix/ts-extension-browser-mime branch from 2b6e6ce to 035e379 Compare July 21, 2026 20:38

kwakayama force-pushed the fix/strip-server-exports-from-client-bundle branch from 983734c to e54e86e Compare July 21, 2026 22:39

kwakayama force-pushed the fix/ts-extension-browser-mime branch from 498bd02 to 884f941 Compare July 21, 2026 22:39

Base automatically changed from fix/ts-extension-browser-mime to main July 21, 2026 22:46

kwakayama force-pushed the fix/strip-server-exports-from-client-bundle branch from e54e86e to bc44e25 Compare July 21, 2026 22:47

kwakayama approved these changes Jul 21, 2026

View reviewed changes

kwakayama enabled auto-merge (squash) July 21, 2026 22:54

kwakayama force-pushed the fix/strip-server-exports-from-client-bundle branch from bc44e25 to 7f73c67 Compare July 21, 2026 22:57

kwakayama approved these changes Jul 21, 2026

View reviewed changes

kojiwakayama approved these changes Jul 21, 2026

View reviewed changes

kwakayama merged commit 63569d8 into main Jul 21, 2026
28 checks passed

kwakayama deleted the fix/strip-server-exports-from-client-bundle branch July 21, 2026 23:37

kwakayama mentioned this pull request Jul 21, 2026

fix(hydration): stop the /index.js fallback from masking real load errors (depends on #2999) #3003

Merged

Navigation Menu

Uh oh!

fix(transforms): strip server-only data hooks from the client artifact (depends on #2999) - #3002

fix(transforms): strip server-only data hooks from the client artifact (depends on #2999)#3002
kwakayama merged 1 commit into
mainfrom
fix/strip-server-exports-from-client-bundle

mattboon commented Jul 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Uh oh!

kwakayama left a comment

Uh oh!

mattboon commented Jul 21, 2026

Uh oh!

kwakayama left a comment

Uh oh!

kwakayama commented Jul 21, 2026

Uh oh!

kwakayama left a comment

Uh oh!

kwakayama left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const stripped = stripFunctionDeclaration(result, name, inert) ??
		stripVariableDeclaration(result, name, inert);

		if (bindings.some((binding) => referenced.has(binding))) continue;
		removals.push({ start: imp.ss, end: imp.se });

Uh oh!

Conversation

mattboon commented Jul 21, 2026

Summary

Reproduction

Test evidence

SSR evidence

Client evidence

Related

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

kwakayama left a comment

Choose a reason for hiding this comment

Uh oh!

mattboon commented Jul 21, 2026

Uh oh!

kwakayama left a comment

Choose a reason for hiding this comment

Uh oh!

kwakayama commented Jul 21, 2026

Uh oh!

kwakayama left a comment

Choose a reason for hiding this comment

Uh oh!

kwakayama left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants