From 882ad7fbbcb3e56f02f2df2c390ceb3475173e69 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Fri, 11 Sep 2026 01:04:17 -0700 Subject: [PATCH] fix(compiler): extract "use server" function declarations nested in a function The bubbling pre-pass turns every function declaration into `const name = function name() {}` at the top of its block so the transform only handles expression forms. Babel skipped the body of each bubbled declaration (`tmp.skip()`), so a declaration nested inside another function was never bubbled, never became an expression, and a directive on it was silently ignored: the body shipped to the client and ran there. The capture validator walks every function, so the same declaration still had its captures rejected. One spelling produced a compile error, the other a client-side body. The bubbler now descends into each bubbled body, so nested declarations are extracted like any other marked function, at any depth and inside nested blocks. The const lands at the top of its block, so a use above the declaration still resolves. Ids follow the binding path (`outer.inner`). Only files that extract at least one function keep the pass's output, so the extra bubbling never reaches a file without server functions. No existing fixture contains a nested declaration, so the frozen references are unchanged. Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- ...act-nested-server-function-declarations.md | 9 + packages/compiler/README.md | 2 +- .../__tests__/directives-id-scheme.test.js | 27 ++- .../directives-nested-declarations.test.js | 155 ++++++++++++++++++ packages/compiler/src/directives/transform.rs | 35 +++- 5 files changed, 215 insertions(+), 13 deletions(-) create mode 100644 .changeset/extract-nested-server-function-declarations.md create mode 100644 packages/compiler/__tests__/directives-nested-declarations.test.js diff --git a/.changeset/extract-nested-server-function-declarations.md b/.changeset/extract-nested-server-function-declarations.md new file mode 100644 index 000000000..d060f2ce6 --- /dev/null +++ b/.changeset/extract-nested-server-function-declarations.md @@ -0,0 +1,9 @@ +--- +"@solidjs/compiler": patch +--- + +A `"use server"` function declaration nested inside another function is now extracted like any other server function. + +- Previously the directive on a nested declaration was silently ignored: the body shipped to the client and ran there, while captures from the enclosing function were still rejected at compile time. +- The declaration is hoisted to a `const` at the top of its block, so calling it before its source position still works. +- Its id follows the binding path, such as `outer.inner`. diff --git a/packages/compiler/README.md b/packages/compiler/README.md index 3286bb021..f263f33ef 100644 --- a/packages/compiler/README.md +++ b/packages/compiler/README.md @@ -153,7 +153,7 @@ result.functions; // [{ id, name, exports }] for manifest building A function-level `"use server"` function is extracted out of its lexical position, so it may only reference its own parameters and locals, module top-level bindings, and globals. Capturing anything else is a compile error. That includes `this` and `arguments` in a marked arrow, which an arrow takes from the function it was written in. Use a `function` if the server function needs its own `this` or `arguments`. -A function-level directive only works where the pass can extract the function: a function declaration, a function expression, or an arrow with a block body. Methods, getters, and setters are never extracted, so a directive on one is a compile error rather than a directive that silently does nothing. Assign a function to a property instead. +A function-level directive only works where the pass can extract the function: a function declaration (at any nesting depth), a function expression, or an arrow with a block body. Methods, getters, and setters are never extracted, so a directive on one is a compile error rather than a directive that silently does nothing. Assign a function to a property instead. A module-level `"use server"` module can only export server functions. Its client build is rebuilt from those exports alone, so anything else would be missing from the browser bundle. Re-exports, `export *`, class and enum exports, destructured exports, and exports declared without an initializer are compile errors naming the export and its position. Type-only and `declare` exports are erased and are fine. diff --git a/packages/compiler/__tests__/directives-id-scheme.test.js b/packages/compiler/__tests__/directives-id-scheme.test.js index f6089ae8a..9f198696a 100644 --- a/packages/compiler/__tests__/directives-id-scheme.test.js +++ b/packages/compiler/__tests__/directives-id-scheme.test.js @@ -299,9 +299,10 @@ describe("the wire id scheme, differentially", () => { }); it("takes a segment from a nested function declaration", () => { - // Declarations nested inside another function are not bubbled into a - // `const`, so the function itself has to contribute the segment, or two - // same-named arrows in sibling declarations collide on `outer.s`. + // Two same-named arrows in sibling declarations must not collide on + // `outer.s`. Reported in post-bubble order (nested declarations hoist in + // reverse source order like top-level ones), which moves the reporting + // order without moving an id. const source = ` export function outer() { function a() { const s = async () => { "use server"; return 1; }; return s; } @@ -310,9 +311,23 @@ describe("the wire id scheme, differentially", () => { } `; const hash = hashHex("src/decl.js"); - expect(ids(source, { filename: "/project/src/decl.js", root: "/project" })).toEqual([ - `outer.a.s-${hash}`, - `outer.b.s-${hash}` + expect(ids(source, { filename: "/project/src/decl.js", root: "/project" }).sort()).toEqual( + [`outer.a.s-${hash}`, `outer.b.s-${hash}`].sort() + ); + }); + + it("names a marked nested function declaration by its path", () => { + // A declaration nested in another function is bubbled into a `const` + // like a top-level one, so it is extracted and named the same way. + const source = ` + export function outer() { + async function inner() { "use server"; return 1; } + return inner; + } + `; + const hash = hashHex("src/nested-decl.js"); + expect(ids(source, { filename: "/project/src/nested-decl.js", root: "/project" })).toEqual([ + `outer.inner-${hash}` ]); }); diff --git a/packages/compiler/__tests__/directives-nested-declarations.test.js b/packages/compiler/__tests__/directives-nested-declarations.test.js new file mode 100644 index 000000000..c7237ab40 --- /dev/null +++ b/packages/compiler/__tests__/directives-nested-declarations.test.js @@ -0,0 +1,155 @@ +// Function declarations nested inside another function carrying a +// function-level `"use server"` directive. +// +// The bubbling pre-pass turns every function declaration into +// `const name = function name() {}` at the top of its block so the transform +// only has to handle expression forms. The Babel original skipped the body of +// each bubbled declaration, so a declaration nested inside another function +// was never bubbled and a directive on it was silently ignored: the body +// shipped to the client and ran there, while the capture validator (which +// walks every function) still rejected its captures. These tests pin that a +// nested declaration is extracted like any other marked function, in both +// modes, and that hoisting still holds after the rewrite. + +const path = require("path"); + +const compilerDir = path.resolve(__dirname, ".."); +const { transformDirectives } = require(compilerDir); + +const RUNTIME = "@solidjs/web/server-functions"; +const ROOT = "/project"; +const FILENAME = `${ROOT}/src/module.js`; + +function compile(code, overrides = {}) { + return transformDirectives(code, { + filename: FILENAME, + root: ROOT, + mode: "server", + env: "production", + directive: "use server", + register: { kind: "named", name: "registerServerReference", source: RUNTIME }, + create: { kind: "named", name: "createServerReference", source: RUNTIME }, + ...overrides + }); +} + +const lines = code => code.split("\n").map(line => line.trim()); + +describe("nested function declarations", () => { + const source = [ + "export function outer() {", + " async function inner() {", + ' "use server";', + " return 1;", + " }", + " return inner;", + "}" + ].join("\n"); + + it("extracts a marked declaration nested in a function (server)", () => { + const result = compile(source); + expect(result.valid).toBe(true); + expect(result.functions.map(f => f.name)).toEqual(["outer.inner"]); + const out = lines(result.code); + // The body is registered at module top level and the binding inside + // `outer` becomes a reference to it, exactly like a top-level function. + expect(out).toContain( + 'const serverFunction_1 = registerServerReference_1("outer.inner-' + + hashOf(result) + + '", async function inner() {' + ); + expect(out).toContain("const inner = createServerReference_1(serverFunction_1);"); + expect(result.code).not.toMatch(/"use server"/); + }); + + it("replaces the declaration with a proxy on the client", () => { + const result = compile(source, { mode: "client" }); + expect(result.valid).toBe(true); + const out = lines(result.code); + expect(out).toContain( + `const inner = createServerReference_1("outer.inner-${hashOf(result)}");` + ); + // The server body is gone from the client build. + expect(result.code).not.toMatch(/return 1/); + expect(result.code).not.toMatch(/registerServerReference/); + }); + + it("keeps the declaration callable before its source position", () => { + // A function declaration is hoisted to the top of its scope. The bubbled + // `const` lands at the top of the block, so a use above the declaration + // still resolves after the rewrite. + const code = [ + "export function outer() {", + " const ref = inner;", + " async function inner() {", + ' "use server";', + " return 1;", + " }", + " return ref;", + "}" + ].join("\n"); + const out = lines(compile(code).code); + const declaration = out.indexOf("const inner = createServerReference_1(serverFunction_1);"); + const use = out.indexOf("const ref = inner;"); + expect(declaration).toBeGreaterThan(-1); + expect(use).toBeGreaterThan(declaration); + }); + + it("extracts through several levels of nesting", () => { + const code = [ + "export function outer() {", + " function mid() {", + ' async function inner() { "use server"; return 1; }', + " return inner;", + " }", + " return mid;", + "}" + ].join("\n"); + const result = compile(code); + expect(result.functions.map(f => f.name)).toEqual(["outer.mid.inner"]); + }); + + it("extracts a declaration inside a block inside a function", () => { + const code = [ + "export function outer(flag) {", + " if (flag) {", + ' async function inner() { "use server"; return 1; }', + " return inner;", + " }", + "}" + ].join("\n"); + const result = compile(code); + expect(result.functions.map(f => f.name)).toEqual(["outer.inner"]); + }); + + it("rejects a nested declaration that captures an enclosing local", () => { + // Extraction and validation agree: the same function that is now + // extracted is the one whose captures were already being checked. + const code = [ + "export function outer(db) {", + ' async function inner() { "use server"; return db.x; }', + " return inner;", + "}" + ].join("\n"); + expect(() => compile(code)).toThrow(/`db` is declared in an enclosing function/); + }); + + it("leaves an unmarked nested declaration's body in place", () => { + const code = [ + "export function outer() {", + " function helper() { return 2; }", + ' const go = async () => { "use server"; return 1; };', + " return [helper, go];", + "}" + ].join("\n"); + const result = compile(code, { mode: "client" }); + expect(result.functions.map(f => f.name)).toEqual(["outer.go"]); + expect(result.code).toMatch(/return 2/); + }); +}); + +// The file hash is whatever the pass produced; the scheme itself is pinned +// differentially in directives-id-scheme.test.js. +function hashOf(result) { + return result.functions[0].id.split("-")[1]; +} diff --git a/packages/compiler/src/directives/transform.rs b/packages/compiler/src/directives/transform.rs index d038ed8c5..187087920 100644 --- a/packages/compiler/src/directives/transform.rs +++ b/packages/compiler/src/directives/transform.rs @@ -702,13 +702,36 @@ impl<'a> DirectivesTransform<'a> { /// Babel's function-level pre-pass bubbles *every* function declaration to a /// `const` at the top of its enclosing block (so exports keep working and the -/// directive transform only has to handle expression forms). Declarations -/// nested inside another bubbled declaration are skipped, matching Babel's -/// `tmp.skip()`. +/// directive transform only has to handle expression forms). +/// +/// Babel skipped the body of each bubbled declaration (`tmp.skip()`), so a +/// declaration nested inside another function was never bubbled, never became +/// an expression, and a `"use server"` directive on it was silently ignored, +/// while the capture validator still treated it as a server function. This +/// pass descends into bubbled bodies too, so nested declarations are +/// extracted like every other marked function. Only files that extract at +/// least one function keep the pass's output, so the extra bubbling never +/// reaches a file without server functions. struct Bubbler<'ctx, 'a> { transform: &'ctx mut DirectivesTransform<'a>, } +impl<'ctx, 'a> Bubbler<'ctx, 'a> { + /// Bubbles the declarations inside `function`'s body, then turns the + /// declaration itself into `const name = function name() {}`. + fn bubble_declaration( + &mut self, + mut function: oxc_allocator::Box<'a, oxc_ast::ast::Function<'a>>, + ) -> Statement<'a> { + walk_mut::walk_function( + self, + &mut function, + oxc_syntax::scope::ScopeFlags::Function, + ); + self.transform.function_declaration_to_const(function) + } +} + impl<'a> VisitMut<'a> for Bubbler<'_, 'a> { fn visit_statements(&mut self, statements: &mut ArenaVec<'a, Statement<'a>>) { let ast = self.transform.ast(); @@ -718,7 +741,7 @@ impl<'a> VisitMut<'a> for Bubbler<'_, 'a> { for statement in old { match statement { Statement::FunctionDeclaration(function) if function.id.is_some() => { - hoisted.push(self.transform.function_declaration_to_const(function)); + hoisted.push(self.bubble_declaration(function)); } Statement::ExportDeclaration(export) if matches!( @@ -731,7 +754,7 @@ impl<'a> VisitMut<'a> for Bubbler<'_, 'a> { unreachable!("shape checked above"); }; let name = function.id.as_ref().unwrap().name.to_string(); - hoisted.push(self.transform.function_declaration_to_const(function)); + hoisted.push(self.bubble_declaration(function)); rest.push( self.transform .export_named_specifier_statement(&name, &name), @@ -753,7 +776,7 @@ impl<'a> VisitMut<'a> for Bubbler<'_, 'a> { unreachable!("shape checked above"); }; let name = function.id.as_ref().unwrap().name.to_string(); - hoisted.push(self.transform.function_declaration_to_const(function)); + hoisted.push(self.bubble_declaration(function)); export.declaration = ExportDefaultDeclarationKind::from(self.transform.identifier(&name)); rest.push(Statement::ExportDefaultDeclaration(export));