Better Response -> BakeResponse transform

This commit is contained in:
Zack Radisic
2025-09-05 20:10:08 -07:00
parent 6d132e628f
commit ee7dfefbe0
4 changed files with 81 additions and 50 deletions

View File

@@ -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(<h1>uh oh</h1>, { 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

View File

@@ -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;

View File

@@ -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 = {

View File

@@ -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");
});