From ee7dfefbe0ff71b49e186c97aefd7fb1a5caf208 Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Fri, 5 Sep 2025 20:10:08 -0700 Subject: [PATCH] Better `Response -> BakeResponse` transform --- src/ast/P.zig | 35 ++++++++++ src/ast/visitExpr.zig | 25 ------- src/bake/hmr-runtime-server.ts | 1 - .../bundler/response-to-bake-response.test.ts | 70 ++++++++++++------- 4 files changed, 81 insertions(+), 50 deletions(-) diff --git a/src/ast/P.zig b/src/ast/P.zig index 226cec0fa9..71cb938714 100644 --- a/src/ast/P.zig +++ b/src/ast/P.zig @@ -170,6 +170,22 @@ pub fn NewParser_( dirname_ref: Ref = Ref.None, import_meta_ref: Ref = Ref.None, hmr_api_ref: Ref = Ref.None, + + /// For SSR we rewrite `Response` -> `SSRResponse` + /// + /// We create a `Response` symbol upfront so we don't accidentally + /// collide with variables declared by the user, i.e. we want to avoid + /// rewriting `Response` in this scenario: + /// + /// ```js + /// export function MyPage() { + /// const Response = 'lmao!'; + /// return new Response(

uh oh

, { status: 200 }); + /// } + /// ``` + response_ref: Ref = Ref.None, + ssr_response_ref: Ref = Ref.None, + scopes_in_order_visitor_index: usize = 0, has_classic_runtime_warned: bool = false, macro_call_count: MacroCallCountType = 0, @@ -2011,6 +2027,17 @@ pub fn NewParser_( .wrap_exports_for_server_reference => {}, } + // Server-side components: + // Declare upfront the symbols for "Response" and "SSRResponse", + // later we'll link "Response" -> "SSRResponse" + switch (p.options.features.server_components) { + .none, .client_side => {}, + else => { + p.response_ref = try p.declareGeneratedSymbol(.class, "Response"); + p.ssr_response_ref = try p.declareGeneratedSymbol(.class, "SSRResponse"); + }, + } + if (p.options.features.hot_module_reloading) { p.hmr_api_ref = try p.declareCommonJSSymbol(.unbound, "hmr"); } @@ -6086,6 +6113,14 @@ pub fn NewParser_( const bundling = p.options.bundle; var parts_end: usize = @as(usize, @intFromBool(bundling)); + if (!p.response_ref.isNull() and !p.ssr_response_ref.isNull()) { + const response_symbol = &p.symbols.items[p.response_ref.innerIndex()]; + // if it has a link it means that the user declared a variable named `Response` + if (!response_symbol.hasLink()) { + response_symbol.link = p.ssr_response_ref; + } + } + // When bundling with HMR, we need every module to be just a // single part, as we later wrap each module into a function, // which requires a single part. Otherwise, you'll end up with diff --git a/src/ast/visitExpr.zig b/src/ast/visitExpr.zig index 405c99f348..7fe130e7f9 100644 --- a/src/ast/visitExpr.zig +++ b/src/ast/visitExpr.zig @@ -92,31 +92,6 @@ pub fn VisitExpr( p.markStrictModeFeature(.reserved_word, js_lexer.rangeOfIdentifier(p.source, expr.loc), name) catch unreachable; } - // Transform Response -> BakeResponse for server-side code - // TODO: this does the incorrect thing in this case: - // ``` - // const Response = 'ooga booga!' - // console.log(Response) - // ``` - if (p.options.features.server_components.isServerSide() and - bun.strings.eqlComptime(name, "Response")) - { - // Find or create BakeResponse identifier - const bake_response_result = p.findSymbol(expr.loc, "SSRResponse") catch unreachable; - e_.ref = bake_response_result.ref; - e_.must_keep_due_to_with_stmt = bake_response_result.is_inside_with_scope; - - std.debug.print("{s}: Response -> SSRResponse\n", .{p.source.path.pretty}); - - // Handle the rest of the identifier processing with the new ref - return p.handleIdentifier(expr.loc, e_, "SSRResponse", IdentifierOpts{ - .assign_target = in.assign_target, - .is_delete_target = is_delete_target, - .is_call_target = @as(Expr.Tag, p.call_target) == .e_identifier and expr.data.e_identifier.ref.eql(p.call_target.e_identifier.ref), - .was_originally_identifier = true, - }); - } - const result = p.findSymbol(expr.loc, name) catch unreachable; e_.must_keep_due_to_with_stmt = result.is_inside_with_scope; diff --git a/src/bake/hmr-runtime-server.ts b/src/bake/hmr-runtime-server.ts index a50c6b86ec..398140bb65 100644 --- a/src/bake/hmr-runtime-server.ts +++ b/src/bake/hmr-runtime-server.ts @@ -76,7 +76,6 @@ server_exports = { const [pageModule, ...layouts] = await Promise.all(routeModules.map(loadExports)); - const mode = pageModule.mode; let requestWithCookies = req; let storeValue: RequestContext = { diff --git a/test/bundler/response-to-bake-response.test.ts b/test/bundler/response-to-bake-response.test.ts index e9c8747f53..2077ac87d3 100644 --- a/test/bundler/response-to-bake-response.test.ts +++ b/test/bundler/response-to-bake-response.test.ts @@ -2,22 +2,22 @@ import { test, expect } from "bun:test"; import { bunEnv, bunExe, tempDirWithFiles } from "harness"; import path from "node:path"; -test("Response -> BakeResponse transform in server components", async () => { +test("Response -> SSRResponse transform in server components", async () => { const dir = tempDirWithFiles("response-transform", { "server-component.js": ` export const mode = "ssr"; export const streaming = false; export default async function ServerPage({ request }) { - // Response should be transformed to BakeResponse + // Response should be transformed to SSRResponse const response1 = new Response("Hello", { status: 200 }); - // Response.redirect should be transformed to BakeResponse.redirect + // Response.redirect should be transformed to SSRResponse.redirect if (!request.userId) { return Response.redirect("/login"); } - // Response.render should be transformed to BakeResponse.render + // Response.render should be transformed to SSRResponse.render if (request.page === "404") { return Response.render("/404"); } @@ -43,11 +43,11 @@ test("Response -> BakeResponse transform in server components", async () => { .env(bunEnv) .text(); - // Check that Response was transformed to BakeResponse in server component + // Check that Response was transformed to SSRResponse in server component expect(serverResult).toContain("SSRResponse"); expect(serverResult).not.toContain("new Response"); - expect(serverResult).toContain("BakeResponse.redirect"); - expect(serverResult).toContain("BakeResponse.render"); + expect(serverResult).toContain("SSRResponse.redirect"); + expect(serverResult).toContain("SSRResponse.render"); // Build client component (should not have the transform) const clientResult = await Bun.$`${bunExe()} build ${path.join(dir, "client-component.js")} --target=browser` @@ -82,9 +82,6 @@ test("Response identifier is transformed in various contexts", async () => { // In destructuring (should not transform if it's a binding) const { Response: LocalResponse } = imports; - // As variable declaration (should not transform) - const Response = MyCustomResponse; - return r1; } `, @@ -96,10 +93,10 @@ test("Response identifier is transformed in various contexts", async () => { await Bun.$`echo ${result} > out.txt`; // Check various contexts - expect(result).toContain("new BakeResponse"); - expect(result).toContain("instanceof BakeResponse"); - expect(result).toContain("BakeResponse.prototype.status"); - expect(result).toContain("BakeResponse.json"); + expect(result).toContain("new SSRResponse"); + expect(result).toContain("instanceof SSRResponse"); + expect(result).toContain("SSRResponse.prototype.status"); + expect(result).toContain("SSRResponse.json"); }); test("Response is not transformed when imported or shadowed", async () => { @@ -111,7 +108,7 @@ test("Response is not transformed when imported or shadowed", async () => { import { Response } from "./custom-response"; export default function Page() { - // Should use the imported Response, not transform to BakeResponse + // Should use the imported Response, not transform to SSRResponse const r = new Response(); return r; } @@ -128,7 +125,7 @@ test("Response is not transformed when imported or shadowed", async () => { return r; } - function inner() { + export function inner() { // But here it should transform since it's not shadowed return new Response(); } @@ -147,18 +144,18 @@ test("Response is not transformed when imported or shadowed", async () => { .text(); // When Response is imported, it should not be transformed - // The bundler will bundle the import, so we check that BakeResponse appears for the global Response + // The bundler will bundle the import, so we check that SSRResponse appears for the global Response // but the imported Response keeps its original behavior - expect(result1).toContain("SSRResponse"); + expect(result1).not.toContain("SSRResponse"); const result2 = await Bun.$`${bunExe()} build ${path.join(dir, "server2.js")} --target=bun --server-components` .env(bunEnv) .text(); // Should preserve local variable - expect(result2).toContain("Response = CustomResponse"); + expect(result2).toContain("return new CustomResponse"); // But the inner function should have the transform - expect(result2).toContain("new BakeResponse"); + expect(result2).toContain("new SSRResponse"); }); test("Response is NOT transformed in client components", async () => { @@ -166,7 +163,7 @@ test("Response is NOT transformed in client components", async () => { "client-component.js": ` "use client"; - // Response should NOT be transformed to BakeResponse in client components + // Response should NOT be transformed to SSRResponse in client components const response = new Response("Client data", { status: 200, headers: { "Content-Type": "text/plain" } @@ -188,7 +185,7 @@ test("Response is NOT transformed in client components", async () => { "server-component.js": ` export const mode = "ssr"; - // This should be transformed to BakeResponse in server component + // This should be transformed to SSRResponse in server component const serverResponse = new Response("Server", { status: 200 }); // Response static methods should be transformed @@ -203,7 +200,7 @@ test("Response is NOT transformed in client components", async () => { .env(bunEnv as any) .text(); - // Verify Response is NOT transformed to BakeResponse in client components + // Verify Response is NOT transformed to SSRResponse in client components expect(clientResult).toContain("new Response"); expect(clientResult).toContain("Response.json"); expect(clientResult).toContain("instanceof Response"); @@ -216,7 +213,32 @@ test("Response is NOT transformed in client components", async () => { .env(bunEnv as any) .text(); - // Server component should have BakeResponse + // Server component should have SSRResponse expect(serverResult).toContain("SSRResponse"); expect(serverResult).not.toContain("new Response"); }); + +test("Response is NOT transformed when Response is shadowed", async () => { + const dir = tempDirWithFiles("response-shadowing", { + "server-component.js": ` + export const mode = "ssr"; + + export function inner() { + const Response = 'ooga booga!'; + const foo = new Response('test', { status: 200 }); + return foo; + } + + export const lmao = new Response() + `, + }); + + // Test 2: Server component - Response SHOULD be transformed + const serverResult = + await Bun.$`${bunExe()} build ${path.join(dir, "server-component.js")} --target=bun --server-components` + .env(bunEnv as any) + .text(); + + expect(serverResult).toContain('return new "ooga booga!"'); + expect(serverResult).toContain("var lmao = new SSRResponse"); +});