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..cdb170a3a 100644 --- a/packages/server/test/seed/action-seed-unit.test.js +++ b/packages/server/test/seed/action-seed-unit.test.js @@ -72,7 +72,7 @@ test('a star re-export is FACETED, not passed through (#1155)', () => { `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'); }); @@ -93,7 +93,7 @@ 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', () => { @@ -130,7 +130,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 }); + } +});