fix(build): strip only the matching declarator in combined export const (#1972)

[Divkix]

Problem

Fixes #1972.

stripServerExports (the client-bundle transform that removes server-only data exports — getServerSideProps / getStaticProps / getStaticPaths) had a correctness bug in its VariableDeclaration branch. When a server export was one declarator inside a multi-declarator export const … statement, the code overwrote the entire statement with a single stub. Two failures resulted:

1. Sibling bindings were deleted. export const myData = 42, getStaticProps = … became export const getStaticProps = undefined; — myData vanished from the client bundle, producing myData is not defined at hydration.
2. Two server exports in one declaration collapsed to one. export const getStaticProps = …, getStaticPaths = … called s.overwrite(node.start, node.end, …) twice on the identical range; MagicString 0.30.21 silently keeps only the last write, emitting a single stub.

Next.js handles this correctly — its SWC transform (next_ssg.rs) marks only the matched declarator's name as Pat::Invalid then prunes it with decls.retain(...), keeping siblings, and the legacy Babel plugin calls per-declarator d.remove(). Next.js even ships a dedicated fixture (should-not-remove-extra-named-export-variable-declarations). So this was a vinext-only parity defect.

Fix

Overwrite only the matched declarator's own range (id = init) instead of the whole statement:

// before
s.overwrite(node.start, node.end, `export const ${declarator.id.name} = undefined;`);
// after
s.overwrite(declarator.start, declarator.end, `${declarator.id.name} = undefined`);

Declarator ranges are disjoint and exclude the const/let/var keyword and the separating commas, so each match is rewritten independently — siblings (including destructuring patterns, whose id is not an Identifier) are preserved, and two server exports in one declaration each get their own stub with no overlapping write. For the single-declarator case the output is byte-for-byte identical to before, so existing behavior and tests are unchanged. As a bonus the declaration kind is now preserved (export let getStaticPaths no longer becomes const).

Tests

Three regression tests added to the stripServerExports block in tests/build-optimization.test.ts (the first ports Next.js babel-plugin-next-ssg-transform.test.ts "should not remove extra named export variable declarations"):

• sibling binding preserved in a combined export const
• two server exports in one declaration each stubbed (no collapse)
• destructuring sibling preserved alongside a server export

All confirmed failing before the fix and passing after. Full unit suite passes (vp test run --project unit), and vp run build && vp run check is clean.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2065

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

commit: 1f15b06

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Reviewed the fix and ran the targeted suite (vp test run tests/build-optimization.test.ts -t "stripServerExports" — 22 passed). The change is correct, minimal, and matches Next.js's next_ssg.rs / Babel behavior of pruning only the matched declarator.

I verified the key cases directly against the transform:

• export const foo = 2, getStaticProps = … → export const foo = 2, getStaticProps = undefined; (sibling preserved)
• two server exports in one statement → both stubbed independently, no comma corruption
• export const { slug } = params, getStaticProps = … → destructuring sibling preserved
• single-declarator case → byte-for-byte identical output (export const getStaticProps = undefined;), so the existing test at line 2772 still holds
• declaration kind preserved (export let getStaticPaths = undefined)

The rationale comment is excellent — it captures both failure modes and the MagicString last-write-wins gotcha, which will save future readers a lot of time.

Two non-blocking observations below; neither needs to be addressed in this PR. Looks good to merge.

[ask-bonk]

Behavioral note (non-blocking, matches Next.js so no change needed): if a sibling references the stripped declarator in the same statement — e.g. export const getStaticProps = 1, bar = getStaticProps; — bar now resolves to undefined rather than its original value. This mirrors Next.js's SSG transform (the binding is stubbed, not just the export), and such intra-statement references to a server data export are exotic, so this is the correct behavior. Flagging only for awareness.

[ask-bonk]

Optional: the three new tests assert result contains substrings like foo = 2 and getStaticProps = undefined, which is good, but they don't pin the exact comma-joined output. Since the original bug was specifically about MagicString collapsing/dropping declarators across commas, a single exact-string assertion on the rewritten statement (e.g. expect(result).toContain("export const foo = 2, getStaticProps = undefined;")) would lock in that the comma separators survive intact. The parseAst round-trip already guards against syntactic corruption, so this is purely a nice-to-have for regression precision.

[ask-bonk]

Review posted successfully. Here's my assessment of PR #2065:

Summary

The fix is correct, minimal, and well-tested. It addresses a genuine vinext-only parity defect in stripServerExports.

The bug: When a server export (getStaticProps/getStaticPaths/getServerSideProps) was one declarator among siblings in a multi-declarator export const, the old code called s.overwrite(node.start, node.end, …) on the whole statement, which:

1. Deleted sibling bindings (export const a = 1, gSP = … lost a)
2. Collapsed two server exports in one declaration to a single stub (MagicString silently keeps only the last write to an identical range)

The fix: Overwrite only the matched declarator's own id = init range. Declarator ranges are disjoint and exclude the keyword/commas, so each match is rewritten independently.

Verification

I ran the transform directly against the relevant cases and ran the targeted suite (22 passed):

• Sibling preserved ✓
• Two server exports each stubbed, no comma corruption ✓
• Destructuring sibling preserved ✓
• Single-declarator output byte-for-byte identical (existing test at line 2772 holds) ✓
• Declaration kind preserved (export let no longer becomes const) ✓

Feedback posted

• Approving review (COMMENT event — fork reviewers can't formally approve).
• Two non-blocking inline notes: one on the intra-statement-reference edge case (matches Next.js, no change needed), and one optional suggestion to add an exact-string assertion to lock in comma-separator survival.

The PR is ready to merge.

github run

[james-elicx]

I'm going to close this as the plugin has been significantly changed via #2055