diff --git a/mordant-baseline.toml b/mordant-baseline.toml index 51d21e458591..44487902a156 100644 --- a/mordant-baseline.toml +++ b/mordant-baseline.toml @@ -45,7 +45,6 @@ "error_collapsed_to_bool:src/runtime/api/bun/h2_frame_parser.rs" = 32 "error_collapsed_to_bool:src/runtime/ffi/ffi_body.rs" = 1 "field_valid_only_when:src/runtime/shell/builtin/rm.rs" = 1 -"reimplemented_helper:src/runtime/api/bun/Terminal.rs" = 1 "reimplemented_helper:src/runtime/hw_exports.rs" = 1 "same_match_twice:src/runtime/api/bun/h2_frame_parser.rs" = 1 "same_match_twice:src/runtime/cli/pack_command.rs" = 1 diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 9bfbf3e85c18..0ab5f8c7bbfc 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -1525,7 +1525,8 @@ impl JSValue { /// `JSValue.getOptional` — loose, coercing property fetch. /// Absent / `undefined` / `null` → `None`; anything else is run through - /// [`coerce`](Self::coerce) (ToNumber for integer `T`). Distinct from + /// [`coerce`](Self::coerce) (ToNumber for integer `T`, returned as-is for + /// `T = JSValue`). Distinct from /// [`get_optional_int`], which validates the property is already an /// in-range integer and throws otherwise. pub fn get_optional( @@ -1900,6 +1901,14 @@ impl PutKey for &str { pub trait CoerceTo: Sized { fn coerce_from(v: JSValue, global: &JSGlobalObject) -> JsResult; } +/// Identity, so `get_optional::` is "the property unless it is +/// absent, `undefined` or `null`" (Zig's `getOptional(.., JSValue)`). +impl CoerceTo for JSValue { + #[inline] + fn coerce_from(v: JSValue, _global: &JSGlobalObject) -> JsResult { + Ok(v) + } +} impl CoerceTo for i32 { fn coerce_from(v: JSValue, global: &JSGlobalObject) -> JsResult { // Fast-path numbers via diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index e181da0fbb13..82d19c88ddec 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -224,30 +224,6 @@ impl Default for Options { } } -// Local extension shims for typed optional property reads. Typed -// `getOptional` is not yet a single inherent generic on `bun_jsc::JSValue`; -// these wrap `get` + the per-type coercion. `withAsyncContextIfNeeded` is the -// inherent `JSValue::with_async_context_if_needed` in `bun_jsc` — call sites -// resolve to that directly, no shim here. -trait JSValueTerminalExt { - fn get_optional_i32(self, global: &JSGlobalObject, name: &[u8]) -> JsResult>; - fn get_optional_value(self, global: &JSGlobalObject, name: &[u8]) -> JsResult>; -} -impl JSValueTerminalExt for JSValue { - fn get_optional_i32(self, global: &JSGlobalObject, name: &[u8]) -> JsResult> { - match self.get(global, name)? { - Some(v) if !v.is_undefined_or_null() => Ok(Some(v.coerce::(global)?)), - _ => Ok(None), - } - } - fn get_optional_value(self, global: &JSGlobalObject, name: &[u8]) -> JsResult> { - match self.get(global, name)? { - Some(v) if !v.is_undefined_or_null() => Ok(Some(v)), - _ => Ok(None), - } - } -} - impl Options { /// Maximum length for terminal name (e.g., "xterm-256color") /// Longest known terminfo names are ~23 chars; 128 allows for custom terminals @@ -261,13 +237,13 @@ impl Options { let mut options = Options::default(); // errdefer options.deinit() — handled by Drop on early return. - if let Some(n) = js_options.get_optional_i32(global_object, b"cols")? { + if let Some(n) = js_options.get_optional::(global_object, b"cols")? { if n > 0 && n <= 65535 { options.cols = u16::try_from(n).expect("int cast"); } } - if let Some(n) = js_options.get_optional_i32(global_object, b"rows")? { + if let Some(n) = js_options.get_optional::(global_object, b"rows")? { if n > 0 && n <= 65535 { options.rows = u16::try_from(n).expect("int cast"); } @@ -284,19 +260,19 @@ impl Options { options.term_name = slice; } - if let Some(v) = js_options.get_optional_value(global_object, b"data")? { + if let Some(v) = js_options.get_optional::(global_object, b"data")? { if v.is_cell() && v.is_callable() { options.data_callback = Some(v.with_async_context_if_needed(global_object)); } } - if let Some(v) = js_options.get_optional_value(global_object, b"exit")? { + if let Some(v) = js_options.get_optional::(global_object, b"exit")? { if v.is_cell() && v.is_callable() { options.exit_callback = Some(v.with_async_context_if_needed(global_object)); } } - if let Some(v) = js_options.get_optional_value(global_object, b"drain")? { + if let Some(v) = js_options.get_optional::(global_object, b"drain")? { if v.is_cell() && v.is_callable() { options.drain_callback = Some(v.with_async_context_if_needed(global_object)); } diff --git a/src/runtime/bake/bake_body.rs b/src/runtime/bake/bake_body.rs index d51f80223766..39ef61c5a63f 100644 --- a/src/runtime/bake/bake_body.rs +++ b/src/runtime/bake/bake_body.rs @@ -68,18 +68,6 @@ fn get_boolean_loose( } } -/// `JSValue.getOptional(JSValue, ..)` — local shim: filters undefined/null. -fn get_optional_value( - target: JSValue, - global: &JSGlobalObject, - property: &[u8], -) -> JsResult> { - match target.get(global, property)? { - Some(v) if !v.is_undefined_or_null() => Ok(Some(v)), - _ => Ok(None), - } -} - /// `JSValue.getFunction` — local shim until `bun_jsc` grows it. fn get_function( target: JSValue, @@ -204,14 +192,14 @@ impl UserOptions { ); } - if let Some(js_options) = get_optional_value(config, global, b"bundlerOptions")? { - if let Some(server_options) = get_optional_value(js_options, global, b"server")? { + if let Some(js_options) = config.get_optional::(global, b"bundlerOptions")? { + if let Some(server_options) = js_options.get_optional::(global, b"server")? { bundler_options.server = BuildConfigSubset::from_js(global, server_options)?; } - if let Some(client_options) = get_optional_value(js_options, global, b"client")? { + if let Some(client_options) = js_options.get_optional::(global, b"client")? { bundler_options.client = BuildConfigSubset::from_js(global, client_options)?; } - if let Some(ssr_options) = get_optional_value(js_options, global, b"ssr")? { + if let Some(ssr_options) = js_options.get_optional::(global, b"ssr")? { bundler_options.ssr = BuildConfigSubset::from_js(global, ssr_options)?; } } @@ -409,7 +397,7 @@ impl BuildConfigSubset { let mut options = BuildConfigSubset::default(); 'brk: { - let Some(val) = get_optional_value(js_options, global, b"sourcemap")? else { + let Some(val) = js_options.get_optional::(global, b"sourcemap")? else { break 'brk; }; if let Some(sourcemap) = source_map_mode_from_js(global, val)? { @@ -426,7 +414,8 @@ impl BuildConfigSubset { } 'brk: { - let Some(minify_options) = get_optional_value(js_options, global, b"minify")? else { + let Some(minify_options) = js_options.get_optional::(global, b"minify")? + else { break 'brk; }; if minify_options.is_boolean() && minify_options.as_boolean() { @@ -837,7 +826,7 @@ impl Framework { Some(ServerComponents { separate_ssr_graph: 'brk: { // Intentionally not using a truthiness check - let prop = match get_optional_value(sc, global, b"separateSSRGraph")? { + let prop = match sc.get_optional::(global, b"separateSSRGraph")? { Some(p) => p, None => { return Err(global.throw_invalid_arguments(format_args!( @@ -1094,7 +1083,7 @@ impl Framework { built_in_modules, }; - if let Some(plugin_array) = get_optional_value(opts, global, b"plugins")? { + if let Some(plugin_array) = opts.get_optional::(global, b"plugins")? { bundler_options.parse_plugin_array(plugin_array, global)?; } diff --git a/test/bake/app-options.test.ts b/test/bake/app-options.test.ts new file mode 100644 index 000000000000..26d0d7b2e3db --- /dev/null +++ b/test/bake/app-options.test.ts @@ -0,0 +1,68 @@ +// Parsing of the `app` option shared by `Bun.serve({ app })` and `bun build --app` +// (src/runtime/bake/bake_body.rs). The optional keys read here treat `null` the +// same as a missing key, while any other value, including `false`, is validated. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("optional app keys treat null as absent", async () => { + using dir = tempDir("bake-app-options", { + "server.ts": `export function render() { return new Response("unused"); }`, + "check.ts": ` + import path from "node:path"; + const framework = { + fileSystemRouterTypes: [ + { root: path.join(import.meta.dir, "routes"), style: "nextjs-pages", serverEntryPoint: "./server.ts" }, + ], + }; + const serverComponents = value => ({ framework: { ...framework, serverComponents: { separateSSRGraph: value } } }); + const apps = { + "bundlerOptions: null": { framework, bundlerOptions: null }, + "bundlerOptions.{server,client,ssr}: null": { framework, bundlerOptions: { server: null, client: null, ssr: null } }, + "bundlerOptions.client.sourcemap: null": { framework, bundlerOptions: { client: { sourcemap: null } } }, + "bundlerOptions.client.sourcemap: 0": { framework, bundlerOptions: { client: { sourcemap: 0 } } }, + "bundlerOptions.client.minify: null": { framework, bundlerOptions: { client: { minify: null } } }, + "framework.plugins: null": { framework: { ...framework, plugins: null } }, + "serverComponents.separateSSRGraph: null": serverComponents(null), + "serverComponents.separateSSRGraph: 0": serverComponents(0), + // false is a value, not "absent": parsing moves on to the next required key. + "serverComponents.separateSSRGraph: false": serverComponents(false), + }; + + const results = {}; + for (const [name, app] of Object.entries(apps)) { + try { + const server = Bun.serve({ port: 0, development: true, app, fetch: () => new Response("") }); + server.stop(true); + results[name] = "accepted"; + } catch (e) { + results[name] = e.message; + } + } + console.log(JSON.stringify(results)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "check.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + "bundlerOptions: null": "accepted", + "bundlerOptions.{server,client,ssr}: null": "accepted", + "bundlerOptions.client.sourcemap: null": "accepted", + "bundlerOptions.client.sourcemap: 0": + 'The "sourcemap" property must be of type "inline" | "external" | "linked", got number', + "bundlerOptions.client.minify: null": "accepted", + "framework.plugins: null": "accepted", + "serverComponents.separateSSRGraph: null": "Missing 'framework.serverComponents.separateSSRGraph'", + "serverComponents.separateSSRGraph: 0": "'framework.serverComponents.separateSSRGraph' must be a boolean", + "serverComponents.separateSSRGraph: false": "Missing 'framework.serverComponents.serverRuntimeImportSource'", + }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/bun/terminal/terminal-spawn.test.ts b/test/js/bun/terminal/terminal-spawn.test.ts index fcb474ffe51a..8ee2c65b8ef4 100644 --- a/test/js/bun/terminal/terminal-spawn.test.ts +++ b/test/js/bun/terminal/terminal-spawn.test.ts @@ -268,6 +268,44 @@ describe("Bun.Terminal subprocess integration", () => { expect(output).toContain("rows=45"); }); + async function sizeSeenBySubprocess(size: Pick): Promise { + let output = ""; + const { promise, resolve, reject } = Promise.withResolvers(); + + // Inline terminal: the subprocess owns it, so once the child exits the reader + // sees EOF (after any output) and `exit` fires. A terminal created up front and + // passed in stays open after the child exits, and `exit` would never run. + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", "process.stdout.write('[' + process.stdout.columns + 'x' + process.stdout.rows + ']')"], + env: bunEnv, + terminal: { + ...size, + data(_term, chunk: Uint8Array) { + output += new TextDecoder().decode(chunk); + // Match the whole token: ConPTY's own escape sequences contain "]" too. + const seen = output.match(/\[\d+x\d+\]/); + if (seen) resolve(seen[0]); + }, + exit() { + reject(new Error("terminal closed before reporting a size; output=" + JSON.stringify(output))); + }, + }, + }); + + const seen = await promise; + await proc.exited; + proc.terminal!.close(); + return seen; + } + + test("null cols/rows leave the default size in place", async () => { + expect(await sizeSeenBySubprocess({ cols: null, rows: null } as any)).toBe("[80x24]"); + }); + + test("cols/rows given as numeric strings are coerced", async () => { + expect(await sizeSeenBySubprocess({ cols: "101", rows: "31" } as any)).toBe("[101x31]"); + }); + test("exit callback fires after close", async () => { const { promise, resolve } = Promise.withResolvers(); const terminal = new Bun.Terminal({