Skip to content

Validate app.bundlerOptions types in Bun.serve #30423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
+59 −5
Closed

Validate app.bundlerOptions types in Bun.serve #30423

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d39cbe0
Validate app.bundlerOptions types in Bun.serve
robobun May 9, 2026
e09316e
ci: retrigger
robobun May 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/bake/bake.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,17 @@ pub const UserOptions = struct {
}

if (try config.getOptional(global, "bundlerOptions", JSValue)) |js_options| {
if (!js_options.isObject()) {
return global.throwInvalidArguments("'bundlerOptions' must be an object", .{});
}
if (try js_options.getOptional(global, "server", JSValue)) |server_options| {
bundler_options.server = try BuildConfigSubset.fromJS(global, server_options);
bundler_options.server = try BuildConfigSubset.fromJS(global, server_options, "server");
}
if (try js_options.getOptional(global, "client", JSValue)) |client_options| {
bundler_options.client = try BuildConfigSubset.fromJS(global, client_options);
bundler_options.client = try BuildConfigSubset.fromJS(global, client_options, "client");
}
if (try js_options.getOptional(global, "ssr", JSValue)) |ssr_options| {
bundler_options.ssr = try BuildConfigSubset.fromJS(global, ssr_options);
bundler_options.ssr = try BuildConfigSubset.fromJS(global, ssr_options, "ssr");
}
}

Expand Down Expand Up @@ -202,9 +205,13 @@ const BuildConfigSubset = struct {
minify_identifiers: ?bool = null,
minify_whitespace: ?bool = null,

pub fn fromJS(global: *jsc.JSGlobalObject, js_options: JSValue) bun.JSError!BuildConfigSubset {
pub fn fromJS(global: *jsc.JSGlobalObject, js_options: JSValue, comptime property_name: []const u8) bun.JSError!BuildConfigSubset {
var options = BuildConfigSubset{};

if (!js_options.isObject()) {
return global.throwInvalidArguments("'bundlerOptions." ++ property_name ++ "' must be an object", .{});
}

if (try js_options.getOptional(global, "sourcemap", JSValue)) |val| brk: {
if (try bun.schema.api.SourceMapMode.fromJS(global, val)) |sourcemap| {
options.source_map = sourcemap;
Expand All @@ -215,13 +222,17 @@ const BuildConfigSubset = struct {
}

if (try js_options.getOptional(global, "minify", JSValue)) |minify_options| brk: {
if (minify_options.isBoolean() and minify_options.asBoolean()) {
if (minify_options.isBoolean()) {
options.minify_syntax = minify_options.asBoolean();
options.minify_identifiers = minify_options.asBoolean();
options.minify_whitespace = minify_options.asBoolean();
break :brk;
}

if (!minify_options.isObject()) {
return global.throwInvalidArguments("'bundlerOptions." ++ property_name ++ ".minify' must be a boolean or an object", .{});
}

if (try minify_options.getBooleanLoose(global, "whitespace")) |value| {
options.minify_whitespace = value;
}
Expand Down
43 changes: 43 additions & 0 deletions test/bake/bundler-options-validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { describe, expect, test } from "bun:test";

describe("Bun.serve app.bundlerOptions validation", () => {
test("non-object bundlerOptions throws", () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: 1225 },
}),
).toThrow("'bundlerOptions' must be an object");
});

for (const key of ["server", "client", "ssr"] as const) {
test(`non-object bundlerOptions.${key} throws`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: 1225 } },
}),
).toThrow(`'bundlerOptions.${key}' must be an object`);
});

test(`non-object non-boolean bundlerOptions.${key}.minify throws`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: { minify: 1225 } } },
}),
).toThrow(`'bundlerOptions.${key}.minify' must be a boolean or an object`);
});

for (const minify of [true, false, { whitespace: true }]) {
test(`bundlerOptions.${key}.minify = ${JSON.stringify(minify)} is accepted`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: { minify } } },
}),
).not.toThrow(/must be/);
});
Comment on lines +33 to +40

@coderabbitai coderabbitai Bot May 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Strengthen accepted-shape assertions to avoid false positives.

At Line 39, .not.toThrow(/must be/) still passes if Bun.serve() throws a different error for an unrelated reason. Assert a specific downstream error to prove this validation branch was actually accepted.

Proposed fix 
-        ).not.toThrow(/must be/);
+        ).toThrow("'app' is missing 'framework'");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test(`bundlerOptions.${key}.minify = ${JSON.stringify(minify)} is accepted`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: { minify } } },
}),
).not.toThrow(/must be/);
});
test(`bundlerOptions.${key}.minify = ${JSON.stringify(minify)} is accepted`, () => {
expect(() =>
Bun.serve({
// `@ts-expect-error`
app: { bundlerOptions: { [key]: { minify } } },
}),
).toThrow("'app' is missing 'framework'");
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/bake/bundler-options-validation.test.ts` around lines 33 - 40, The test
currently uses .not.toThrow(/must be/) which can pass for unrelated errors;
instead assert for the specific downstream error that occurs only when the
bundlerOptions shape is accepted. Update the assertion in the test around
Bun.serve(...) (the block creating app: { bundlerOptions: { [key]: { minify } }
}) to expect the known runtime/startup error message produced when the
bundlerOptions branch is accepted (e.g., replace .not.toThrow(/must be/) with
.toThrow(/<actual downstream error message>/)); run the test once to capture the
real downstream message and use that exact regex so the test proves this
validation path was exercised.

}
}
Comment on lines +13 to +42

@coderabbitai coderabbitai Bot May 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Use describe.each() for parameterized cases.

The loop-based matrix works, but this suite should use describe.each() per repo test conventions for parameterized coverage and reporting clarity.

Refactor sketch 
-  for (const key of ["server", "client", "ssr"] as const) {
-    test(`non-object bundlerOptions.${key} throws`, () => {
+  describe.each(["server", "client", "ssr"] as const)("bundlerOptions.%s", key => {
+    test("non-object throws", () => {
       ...
-    test(`non-object non-boolean bundlerOptions.${key}.minify throws`, () => {
+    test("non-object non-boolean minify throws", () => {
       ...
-    for (const minify of [true, false, { whitespace: true }]) {
-      test(`bundlerOptions.${key}.minify = ${JSON.stringify(minify)} is accepted`, () => {
+    describe.each([true, false, { whitespace: true }] as const)("minify=%j", minify => {
+      test("is accepted", () => {
         ...
       });
-    }
-  }
+    });
+  });

As per coding guidelines: "Use describe.each() for parameterized tests".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const key of ["server", "client", "ssr"] as const) {
test(`non-object bundlerOptions.${key} throws`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: 1225 } },
}),
).toThrow(`'bundlerOptions.${key}' must be an object`);
});
test(`non-object non-boolean bundlerOptions.${key}.minify throws`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: { minify: 1225 } } },
}),
).toThrow(`'bundlerOptions.${key}.minify' must be a boolean or an object`);
});
for (const minify of [true, false, { whitespace: true }]) {
test(`bundlerOptions.${key}.minify = ${JSON.stringify(minify)} is accepted`, () => {
expect(() =>
Bun.serve({
// @ts-expect-error
app: { bundlerOptions: { [key]: { minify } } },
}),
).not.toThrow(/must be/);
});
}
}
describe.each(["server", "client", "ssr"] as const)("bundlerOptions.%s", key => {
test("non-object throws", () => {
expect(() =>
Bun.serve({
// `@ts-expect-error`
app: { bundlerOptions: { [key]: 1225 } },
}),
).toThrow(`'bundlerOptions.${key}' must be an object`);
});
test("non-object non-boolean minify throws", () => {
expect(() =>
Bun.serve({
// `@ts-expect-error`
app: { bundlerOptions: { [key]: { minify: 1225 } } },
}),
).toThrow(`'bundlerOptions.${key}.minify' must be a boolean or an object`);
});
describe.each([true, false, { whitespace: true }] as const)("minify=%j", minify => {
test("is accepted", () => {
expect(() =>
Bun.serve({
// `@ts-expect-error`
app: { bundlerOptions: { [key]: { minify } } },
}),
).not.toThrow(/must be/);
});
});
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/bake/bundler-options-validation.test.ts` around lines 13 - 42, Replace
the manual for-loops with Jest's parameterized helpers: use describe.each([...])
for the outer matrix over ["server","client","ssr"] to create a describe block
per key, and inside each describe use either test.each([...]) or another
describe.each for the minify variants ([true, false, { whitespace: true }]) to
produce individual tests for non-object cases and accepted values; update the
test titles to interpolate the current key/minify (matching existing strings
like `non-object bundlerOptions.${key} throws`, `non-object non-boolean
bundlerOptions.${key}.minify throws`, and `bundlerOptions.${key}.minify =
${JSON.stringify(minify)} is accepted`) and remove the loop variables, keeping
the same Bun.serve assertions.

});
Loading