Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 110 additions & 8 deletions src/transforms/pipeline/stages/browser-server-exports-strip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,12 +758,7 @@ describe("browser-server-exports-strip", () => {
assertEquals(occurrences(result, "TOKEN"), 0); // hook-only tail → dropped
});

// Known limitation (pinned): a *destructured* server value is NOT pruned —
// `moduleScopeDeclarations` handles only simple identifiers, to avoid
// mishandling default-value references inside patterns. Conservative (never
// over-prunes) but it means a destructured server value still ships. If this
// ever needs closing, extend the declaration collector to safe patterns.
it("conservatively keeps a destructured server value (documented limitation)", async () => {
it("drops a destructured module-scope server value", async () => {
const code = [
`import { getEnv } from "veryfront";`,
`const { a } = getEnv("X");`,
Expand All @@ -773,8 +768,115 @@ describe("browser-server-exports-strip", () => {

const result = await stripServerOnlyExports(code);

// Pinned as-is: the destructured binding and its import survive.
assertStringIncludes(result, "getEnv");
assertNotIncludes(result, "getEnv");
assertEquals(occurrences(result, "a"), 0);
});

it("drops nested, array, and rest bindings used only by the hook", async () => {
const code = [
`import { getEnv } from "veryfront";`,
`const { nested: { value }, list: [first, , ...rest] } = getEnv("SERVER_ONLY");`,
`export async function getServerData() { return { props: { value, first, rest } }; }`,
`export default function Page() { return null; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertNotIncludes(result, "SERVER_ONLY");
assertNotIncludes(result, "getEnv");
assertEquals(occurrences(result, "value"), 0);
assertEquals(occurrences(result, "first"), 0);
assertEquals(occurrences(result, "rest"), 0);
});

it("conservatively keeps a pattern with a default value", async () => {
const code = [
`import { getEnv } from "veryfront";`,
`const DEFAULT = getEnv("CLIENT_FALLBACK");`,
`const { a = DEFAULT } = getEnv("SERVER_ONLY");`,
`export async function getServerData() { return { props: { a } }; }`,
`export default function Page() { return DEFAULT; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertStringIncludes(result, "CLIENT_FALLBACK");
assertStringIncludes(result, "DEFAULT");
assertStringIncludes(result, "SERVER_ONLY");
assertStringIncludes(result, "a = DEFAULT");
});

it("conservatively keeps a pattern with a computed key", async () => {
const code = [
`import { getEnv } from "veryfront";`,
`const KEY = getEnv("CLIENT_KEY");`,
`const { [KEY]: value } = getEnv("SERVER_ONLY");`,
`export async function getServerData() { return { props: { value } }; }`,
`export default function Page() { return KEY; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertStringIncludes(result, "CLIENT_KEY");
assertStringIncludes(result, "KEY");
assertStringIncludes(result, "SERVER_ONLY");
assertStringIncludes(result, "[KEY]: value");
});

it("removes one destructuring declarator without dropping its client sibling", async () => {
const code = [
`import { getEnv } from "veryfront";`,
`const { a } = getEnv("SERVER_ONLY"), client = bootClient();`,
`export async function getServerData() { return { props: { a } }; }`,
`export default function Page() { return client; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertNotIncludes(result, "SERVER_ONLY");
assertNotIncludes(result, "getEnv");
assertStringIncludes(result, "client = bootClient()");
assertStringIncludes(result, "return client");
});

it("keeps a destructuring default with an unrelated client effect", async () => {
const code = [
`const { token, client = startClient() } = loadSecret();`,
`export async function getServerData() { return { props: { token } }; }`,
`export default function Page() { return null; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertStringIncludes(result, "client = startClient()");
assertStringIncludes(result, "loadSecret()");
});

it("keeps a computed pattern key with an unrelated client effect", async () => {
const code = [
`const { [startClient()]: token } = loadSecret();`,
`export async function getServerData() { return { props: { token } }; }`,
`export default function Page() { return null; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertStringIncludes(result, "[startClient()]: token");
assertStringIncludes(result, "loadSecret()");
});

it("keeps a destructuring declarator with a sibling outside the hook closure", async () => {
const code = [
`const { token, client } = loadSecret();`,
`export async function getServerData() { return { props: { token } }; }`,
`export default function Page() { return null; }`,
].join("\n");

const result = await stripServerOnlyExports(code);

assertStringIncludes(result, "loadSecret()");
assertEquals(occurrences(result, "token"), 1);
assertEquals(occurrences(result, "client"), 1);
});

it("keeps an import that the client still references", async () => {
Expand Down
63 changes: 50 additions & 13 deletions src/transforms/pipeline/stages/browser-server-exports-strip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,13 @@ async function parseStubs(parser: CodeParser): Promise<{ body: Node; init: Node
return { body, init };
}

/** Every binding name a destructuring pattern introduces. */
function patternBoundNames(pattern: Node): string[] {
const names: string[] = [];
/** Every binding identifier a variable or parameter pattern introduces. */
function patternBindingIds(pattern: Node): Node[] {
const bindings: Node[] = [];

const collect = (node: Node): void => {
if (node.type === "Identifier") {
const name = nodeName(node);
if (name) names.push(name);
bindings.push(node);
return;
}

Expand Down Expand Up @@ -274,7 +273,40 @@ function patternBoundNames(pattern: Node): string[] {

collect(pattern);

return names;
return bindings;
}

/** Whether evaluating a pattern can run code outside its binding positions. */
function patternHasEvaluatedValuePosition(pattern: Node): boolean {
if (pattern.type === "Identifier") return false;
if (pattern.type === "AssignmentPattern") return true;
if (pattern.type === "RestElement") {
return !isNode(pattern.argument) || patternHasEvaluatedValuePosition(pattern.argument);
}
if (pattern.type === "TSParameterProperty") {
return !isNode(pattern.parameter) || patternHasEvaluatedValuePosition(pattern.parameter);
}
if (pattern.type === "ArrayPattern") {
return (Array.isArray(pattern.elements) ? pattern.elements : []).some((element) =>
isNode(element) && patternHasEvaluatedValuePosition(element)
);
}
if (pattern.type === "ObjectPattern") {
return (Array.isArray(pattern.properties) ? pattern.properties : []).some((property) => {
if (!isNode(property)) return false;
if (property.type === "RestElement") {
return patternHasEvaluatedValuePosition(property);
}
return property.type !== "ObjectProperty" || property.computed === true ||
!isNode(property.value) || patternHasEvaluatedValuePosition(property.value);
});
}
return true;
}

/** Every binding name a destructuring pattern introduces. */
function patternBoundNames(pattern: Node): string[] {
return patternBindingIds(pattern).map(nodeName).filter((name): name is string => Boolean(name));
}

/**
Expand Down Expand Up @@ -489,8 +521,9 @@ interface ModuleScopeDecl {
* Non-exported top-level `const`/`let`/`var`/`function`/`class` declarations
* whose bindings we could safely drop if nothing references them. Exported
* declarations are part of the module's contract and are never candidates.
* Destructuring declarations are skipped — a pattern can carry default-value
* references, and a partial removal is not worth the risk.
* Patterns with defaults or computed keys stay fail-closed because evaluating
* them can run unrelated client code. In supported patterns, only binding
* positions are excluded from liveness analysis.
*/
function moduleScopeDeclarations(body: Node[]): ModuleScopeDecl[] {
const decls: ModuleScopeDecl[] = [];
Expand All @@ -511,13 +544,17 @@ function moduleScopeDeclarations(body: Node[]): ModuleScopeDecl[] {
) {
if (!isNode(declarator)) continue;
const id = declarator.id;
if (isNode(id) && id.type === "Identifier") {
const name = nodeName(id);
if (name) variableDecls.push({ statement, declarator, names: [name], bindingIds: [id] });
} else {
if (!isNode(id) || patternHasEvaluatedValuePosition(id)) {
variableDecls.length = 0;
break;
}
const bindingIds = patternBindingIds(id);
const names = bindingIds.map(nodeName).filter((name): name is string => Boolean(name));
if (names.length === 0 || names.length !== bindingIds.length) {
variableDecls.length = 0;
break;
}
variableDecls.push({ statement, declarator, names, bindingIds });
Comment thread
kojiwakayama marked this conversation as resolved.
}

decls.push(...variableDecls);
Expand Down Expand Up @@ -1164,7 +1201,7 @@ function dropUnusedModuleScopeBindings(body: Node[], hookClosure: Set<string>):
const removableDeclarators = new Map<Node, Set<Node>>();
const removedDecls: ModuleScopeDecl[] = [];
for (const decl of decls) {
const inClosure = decl.names.some((name) => hookClosure.has(name));
const inClosure = decl.names.every((name) => hookClosure.has(name));
const unused = decl.names.every((name) => !referenced.has(name));
if (!inClosure || !unused) continue;

Expand Down
2 changes: 1 addition & 1 deletion templates/manifest.generated.ts

Large diffs are not rendered by default.