From 420bcdbb59814821a3baf286b3028ff465feebe8 Mon Sep 17 00:00:00 2001 From: Vivek Date: Sat, 1 Aug 2026 18:03:06 +0530 Subject: [PATCH 1/2] fix(server): wrap exported functions as hoisted declarations in action seed facade (#1208) --- packages/server/src/action-seed.js | 48 ++++++++++--------- .../server/test/seed/action-seed-unit.test.js | 32 ++----------- packages/server/test/seed/seed-hook.test.js | 29 ++++++++++- test/bun/action-seed-circular.test.mjs | 46 ++++++++++++++++++ 4 files changed, 102 insertions(+), 53 deletions(-) create mode 100644 test/bun/action-seed-circular.test.mjs diff --git a/packages/server/src/action-seed.js b/packages/server/src/action-seed.js index 206c4d840..a9a3c5bc9 100644 --- a/packages/server/src/action-seed.js +++ b/packages/server/src/action-seed.js @@ -283,31 +283,35 @@ function seedProxy(file, fnName, orig) { * here: the facade re-exports it wholesale through its own `export * from` * catch-all (#538), so nothing about it needs a decision. * @param {string} src - * @returns {{ names: string[], hasDefault: boolean } } + * @returns {{ fnNames: string[], valNames: string[], names: string[], hasDefault: boolean }} */ export function extractExportNames(src) { - const names = new Set(); + const fnNames = new Set(); + const valNames = new Set(); let m; const reFn = /\bexport\s+(?:async\s+)?function\s*\*?\s+([A-Za-z_$][\w$]*)/g; - while ((m = reFn.exec(src))) names.add(m[1]); - const reVar = /\bexport\s+(?:const|let|var)\s+([A-Za-z_$][\w$]*)/g; - while ((m = reVar.exec(src))) names.add(m[1]); + while ((m = reFn.exec(src))) fnNames.add(m[1]); + const reFnVar = /\bexport\s+(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:async\s*)?(?:function|\([^)]*\)\s*=>|[\w$]+\s*=>)/g; + while ((m = reFnVar.exec(src))) fnNames.add(m[1]); const reClass = /\bexport\s+(?:abstract\s+)?class\s+([A-Za-z_$][\w$]*)/g; - while ((m = reClass.exec(src))) names.add(m[1]); + while ((m = reClass.exec(src))) valNames.add(m[1]); const reList = /\bexport\s*\{([^}]*)\}/g; while ((m = reList.exec(src))) { for (const part of m[1].split(',')) { const seg = part.trim(); if (!seg) continue; - // `a` or `a as b` (the EXPORTED name is what an importer binds). const as = seg.split(/\s+as\s+/); const exported = (as[1] || as[0]).trim(); - if (/^[A-Za-z_$][\w$]*$/.test(exported) && exported !== 'default') names.add(exported); - else if (exported === 'default') names.add('__default__'); + if (/^[A-Za-z_$][\w$]*$/.test(exported) && exported !== 'default') fnNames.add(exported); + else if (exported === 'default') fnNames.add('__default__'); } } - const hasDefault = /\bexport\s+default\b/.test(src) || names.delete('__default__'); - return { names: [...names], hasDefault }; + const reVar = /\bexport\s+(?:const|let|var)\s+([A-Za-z_$][\w$]*)/g; + while ((m = reVar.exec(src))) { + if (!fnNames.has(m[1])) valNames.add(m[1]); + } + const hasDefault = /\bexport\s+default\b/.test(src) || fnNames.delete('__default__') || valNames.delete('__default__'); + return { fnNames: [...fnNames], valNames: [...valNames], names: [...fnNames, ...valNames], hasDefault }; } /** @@ -316,7 +320,7 @@ export function extractExportNames(src) { * unwrapped) and re-exports each function through `__actionWrap`. * @param {string} origUrl the real module URL, WITHOUT the seed query * @param {string} absPath the real module's absolute file path (the hash basis) - * @param {{ names: string[], hasDefault: boolean }} exports + * @param {{ fnNames: string[], valNames: string[], names: string[], hasDefault: boolean }} exports * @returns {string} */ function buildFacade(origUrl, absPath, exports) { @@ -325,18 +329,16 @@ function buildFacade(origUrl, absPath, exports) { const file = JSON.stringify(absPath); let out = `import * as __orig from ${origSpec};\n`; out += `import { __actionWrap as __w } from ${JSON.stringify(SELF_URL)};\n`; - // Fail-open catch-all (#535). Re-export every named binding of the real module. - // An explicit `export const NAME = __w(...)` below SHADOWS the matching star - // binding (an explicit export wins over a star re-export of the same name, with - // no SyntaxError), so an enumerated export is still wrapped and seeded. A named - // export the `extractExportNames` regex MISSED (exotic syntax, an unusual - // re-export form, a codegen-produced export) is NOT enumerated below, so it - // flows through this star unwrapped: it resolves and works over a normal RPC, - // just is not seeded, instead of being dropped and crashing the importer with - // `undefined`. `export *` does not carry `default`, which the explicit default - // line below still handles. out += `export * from ${origSpec};\n`; - for (const n of exports.names) { + for (const n of exports.fnNames) { + const k = JSON.stringify(n); + out += `export function ${n}(...args) {\n`; + out += ` const fn = __w(${file}, ${k}, __orig[${k}]);\n`; + out += ` return typeof fn === 'function' ? fn.apply(this, args) : fn;\n`; + out += `}\n`; + out += `__w(${file}, ${k}, ${n});\n`; + } + for (const n of exports.valNames) { const k = JSON.stringify(n); out += `export const ${n} = __w(${file}, ${k}, __orig[${k}]);\n`; } diff --git a/packages/server/test/seed/action-seed-unit.test.js b/packages/server/test/seed/action-seed-unit.test.js index 326698356..011726e49 100644 --- a/packages/server/test/seed/action-seed-unit.test.js +++ b/packages/server/test/seed/action-seed-unit.test.js @@ -59,32 +59,18 @@ test('extractExportNames finds function / const / class / list / default exports }); test('a star re-export is FACETED, not passed through (#1155)', () => { - // It used to bail out to a passthrough, on the reasoning that an - // unenumerable re-export must not be silently dropped. #538 then gave the - // facade its own `export * from` catch-all, which covers exactly that, and - // the bail-out was left behind. Keeping it became a correctness bug once - // action IDENTITY started riding the facade: a passthrough means the function - // is never registered, so `
` throws "is not a server - // action" at SSR while the same export works fine over RPC. const src = `'use server';\n` + `export * from './shared.server.js';\n` + `export async function createTodo(fd) { return fd; }\n`; const facade = buildSeedFacade('file:///app/t.server.js', '/app/t.server.js', src); assert.ok(facade, 'a star re-export is faceted like any other module'); - assert.match(facade, /export const createTodo = __w\(/, 'its own export is wrapped, so identity resolves'); + assert.match(facade, /export (?:const|function) createTodo\b/, 'its own export is wrapped, so identity resolves'); assert.match(facade, /export \* from "file:\/\/\/app\/t\.server\.js\?webjs-seed-orig"/, 'the star catch-all carries the re-exported bindings'); }); test('the word "export" in a comment does not change how a module loads', () => { - // This used to be decided by a `/\bexport\s*\*/` test over the raw source, - // and `\s*` spans newlines, so the word `export` ending a JSDoc line matched - // the next line's leading `*` and suppressed the facade. Since identity rides - // the facade, reflowing a doc comment could turn a working bound form into a - // 500 on the page rendering it. There is no such test any more (the star case - // facades like everything else), so the guarantee is now structural, and this - // pins it end to end: prose in, a wrapped export out. const src = `'use server';\n` + `/**\n` + @@ -93,17 +79,10 @@ test('the word "export" in a comment does not change how a module loads', () => `export async function submitFeedback(fd) { return fd; }\n`; const facade = buildSeedFacade('file:///app/f.server.js', '/app/f.server.js', src); assert.ok(facade, 'the module is faceted'); - assert.match(facade, /export const submitFeedback = __w\(/, 'so its identity registers'); + assert.match(facade, /export (?:const|function) submitFeedback\b/, 'so its identity registers'); }); test('a re-exported action keeps the identity of its DEFINING module', () => { - // A barrel (`export { createTodo } from './create.server.js'`) is faceted too, - // and its body evaluates AFTER the module it re-exports from, so an - // unconditional registration re-filed the function under the BARREL. The - // dispatcher would then load the barrel to run it and read `validate` / - // `middleware` / `method` / `invalidates` off a namespace carrying none of - // them, running a form submission with the action's validation and auth - // middleware silently skipped. const real = async () => 'ok'; __actionWrap('/app/modules/todo/actions/create.server.js', 'createTodo', real); __actionWrap('/app/modules/todo/actions/index.server.js', 'createTodo', real); @@ -114,11 +93,6 @@ test('a re-exported action keeps the identity of its DEFINING module', () => { }); test('buildSeedFacade emits an export* catch-all so a MISSED export is fail-open (#535)', () => { - // `export const { BRAND } = ...` is a destructuring export. The - // identifier-after-`const` regex in extractExportNames does NOT match it, so - // BRAND is the canonical "missed" export. Before the catch-all, the facade - // omitted BRAND entirely, so `import { BRAND }` resolved to `undefined` and - // crashed the importer. The facade must now carry BRAND via `export *`. const src = `'use server';\n` + `export async function getUser(id) { return id; }\n` + @@ -130,7 +104,7 @@ test('buildSeedFacade emits an export* catch-all so a MISSED export is fail-open /export \* from "file:\/\/\/app\/x\.server\.js\?webjs-seed-orig"/, 'the facade re-exports everything via a star catch-all (the fail-open guard)', ); - assert.match(facade, /export const getUser = __w\(/, 'an enumerated export is still wrapped + seeded'); + assert.match(facade, /export (?:const|function) getUser\b/, 'an enumerated export is still wrapped + seeded'); assert.doesNotMatch( facade, /export const BRAND =/, diff --git a/packages/server/test/seed/seed-hook.test.js b/packages/server/test/seed/seed-hook.test.js index 044443d1e..bf344d125 100644 --- a/packages/server/test/seed/seed-hook.test.js +++ b/packages/server/test/seed/seed-hook.test.js @@ -21,7 +21,7 @@ import { hashFile } from '../../src/actions.js'; import { stringify } from '@webjsdev/core'; let dir; -let actionUrl, utilUrl, exoticUrl; +let actionUrl, utilUrl, exoticUrl, c1Url, c2Url; before(async () => { dir = mkdtempSync(join(tmpdir(), 'webjs-seedhook-')); @@ -49,6 +49,24 @@ before(async () => { utilUrl = pathToFileURL(util).toString(); exoticUrl = pathToFileURL(exotic).toString(); + // Circular re-export pair (#1208) + const c1 = join(dir, 'c1.server.js'); + const c2 = join(dir, 'c2.server.js'); + writeFileSync( + c1, + `'use server';\n` + + `export { helper } from './c2.server.js';\n` + + `export async function ring(x) { return x + 1; }\n`, + ); + writeFileSync( + c2, + `'use server';\n` + + `export { ring } from './c1.server.js';\n` + + `export async function helper(x) { return x * 2; }\n`, + ); + c1Url = pathToFileURL(c1).toString(); + c2Url = pathToFileURL(c2).toString(); + // Install the global hook BEFORE importing the fixtures (ESM caches by URL). await registerActionHooks({ seed: true }); }); @@ -95,3 +113,12 @@ test('an export the facade regex MISSES flows through the export* catch-all, not assert.equal(value, 40, 'the enumerated action still runs and is seeded'); assert.equal(collector.size, 1, 'the enumerated action seeds; the missed (unwrapped) ones do not'); }); + +test('circular re-export between two use-server modules loads without throwing (#1208)', async () => { + const mod1 = await import(c1Url); + assert.equal(typeof mod1.ring, 'function'); + assert.equal(typeof mod1.helper, 'function'); + assert.equal(await mod1.ring(5), 6); + assert.equal(await mod1.helper(5), 10); +}); + diff --git a/test/bun/action-seed-circular.test.mjs b/test/bun/action-seed-circular.test.mjs new file mode 100644 index 000000000..875d33655 --- /dev/null +++ b/test/bun/action-seed-circular.test.mjs @@ -0,0 +1,46 @@ +/** + * Bun parity test for circular re-exports between 'use server' modules (#1208). + * + * Proves that two 'use server' modules that re-export from each other load + * without throwing ReferenceError on Bun as well as Node. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; + +import { installBunSeedPlugin } from '../../packages/server/src/action-seed-bun.js'; +import { seedingEnabled, registerActionHooks } from '../../packages/server/src/action-seed.js'; + +test('circular re-export between use-server modules loads on Bun / Node (#1208)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'webjs-bun-circular-')); + try { + const c1 = join(dir, 'c1.server.js'); + const c2 = join(dir, 'c2.server.js'); + writeFileSync( + c1, + `'use server';\n` + + `export { helper } from './c2.server.js';\n` + + `export async function ring(x) { return x + 1; }\n`, + ); + writeFileSync( + c2, + `'use server';\n` + + `export { ring } from './c1.server.js';\n` + + `export async function helper(x) { return x * 2; }\n`, + ); + + if (!seedingEnabled()) await registerActionHooks({ seed: true }); + + const c1Url = pathToFileURL(c1).toString(); + const mod1 = await import(c1Url); + assert.equal(typeof mod1.ring, 'function'); + assert.equal(typeof mod1.helper, 'function'); + assert.equal(await mod1.ring(10), 11); + assert.equal(await mod1.helper(10), 20); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); From 76eb282b88a4a3e5faf88c8bdc77cd208eca3b5c Mon Sep 17 00:00:00 2001 From: Vivek Date: Sat, 1 Aug 2026 18:17:28 +0530 Subject: [PATCH 2/2] docs: preserve original test comments in action-seed-unit.test.js --- .../server/test/seed/action-seed-unit.test.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/server/test/seed/action-seed-unit.test.js b/packages/server/test/seed/action-seed-unit.test.js index 011726e49..cdb170a3a 100644 --- a/packages/server/test/seed/action-seed-unit.test.js +++ b/packages/server/test/seed/action-seed-unit.test.js @@ -59,6 +59,13 @@ test('extractExportNames finds function / const / class / list / default exports }); test('a star re-export is FACETED, not passed through (#1155)', () => { + // It used to bail out to a passthrough, on the reasoning that an + // unenumerable re-export must not be silently dropped. #538 then gave the + // facade its own `export * from` catch-all, which covers exactly that, and + // the bail-out was left behind. Keeping it became a correctness bug once + // action IDENTITY started riding the facade: a passthrough means the function + // is never registered, so `` throws "is not a server + // action" at SSR while the same export works fine over RPC. const src = `'use server';\n` + `export * from './shared.server.js';\n` + @@ -71,6 +78,13 @@ test('a star re-export is FACETED, not passed through (#1155)', () => { }); test('the word "export" in a comment does not change how a module loads', () => { + // This used to be decided by a `/\bexport\s*\*/` test over the raw source, + // and `\s*` spans newlines, so the word `export` ending a JSDoc line matched + // the next line's leading `*` and suppressed the facade. Since identity rides + // the facade, reflowing a doc comment could turn a working bound form into a + // 500 on the page rendering it. There is no such test any more (the star case + // facades like everything else), so the guarantee is now structural, and this + // pins it end to end: prose in, a wrapped export out. const src = `'use server';\n` + `/**\n` + @@ -83,6 +97,13 @@ test('the word "export" in a comment does not change how a module loads', () => }); test('a re-exported action keeps the identity of its DEFINING module', () => { + // A barrel (`export { createTodo } from './create.server.js'`) is faceted too, + // and its body evaluates AFTER the module it re-exports from, so an + // unconditional registration re-filed the function under the BARREL. The + // dispatcher would then load the barrel to run it and read `validate` / + // `middleware` / `method` / `invalidates` off a namespace carrying none of + // them, running a form submission with the action's validation and auth + // middleware silently skipped. const real = async () => 'ok'; __actionWrap('/app/modules/todo/actions/create.server.js', 'createTodo', real); __actionWrap('/app/modules/todo/actions/index.server.js', 'createTodo', real); @@ -93,6 +114,11 @@ test('a re-exported action keeps the identity of its DEFINING module', () => { }); test('buildSeedFacade emits an export* catch-all so a MISSED export is fail-open (#535)', () => { + // `export const { BRAND } = ...` is a destructuring export. The + // identifier-after-`const` regex in extractExportNames does NOT match it, so + // BRAND is the canonical "missed" export. Before the catch-all, the facade + // omitted BRAND entirely, so `import { BRAND }` resolved to `undefined` and + // crashed the importer. The facade must now carry BRAND via `export *`. const src = `'use server';\n` + `export async function getUser(id) { return id; }\n` +