jsc: let get_optional::<JSValue> replace the Terminal and bake get_optional_value shims

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

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<T: CoerceTo>(
@@ -1900,6 +1901,14 @@ impl PutKey for &str {
 pub trait CoerceTo: Sized {
     fn coerce_from(v: JSValue, global: &JSGlobalObject) -> JsResult<Self>;
 }
+/// Identity, so `get_optional::<JSValue>` 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<JSValue> {
+        Ok(v)
+    }
+}
 impl CoerceTo for i32 {
     fn coerce_from(v: JSValue, global: &JSGlobalObject) -> JsResult<i32> {
         // Fast-path numbers via

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<Option<i32>>;
-    fn get_optional_value(self, global: &JSGlobalObject, name: &[u8]) -> JsResult<Option<JSValue>>;
-}
-impl JSValueTerminalExt for JSValue {
-    fn get_optional_i32(self, global: &JSGlobalObject, name: &[u8]) -> JsResult<Option<i32>> {
-        match self.get(global, name)? {
-            Some(v) if !v.is_undefined_or_null() => Ok(Some(v.coerce::<i32>(global)?)),
-            _ => Ok(None),
-        }
-    }
-    fn get_optional_value(self, global: &JSGlobalObject, name: &[u8]) -> JsResult<Option<JSValue>> {
-        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::<i32>(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::<i32>(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::<JSValue>(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::<JSValue>(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::<JSValue>(global_object, b"drain")? {
             if v.is_cell() && v.is_callable() {
                 options.drain_callback = Some(v.with_async_context_if_needed(global_object));
             }

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<Option<JSValue>> {
-    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::<JSValue>(global, b"bundlerOptions")? {
+            if let Some(server_options) = js_options.get_optional::<JSValue>(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::<JSValue>(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::<JSValue>(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::<JSValue>(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::<JSValue>(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::<JSValue>(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::<JSValue>(global, b"plugins")? {
             bundler_options.parse_plugin_array(plugin_array, global)?;
         }
 

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);
+});

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<Bun.TerminalOptions, "cols" | "rows">): Promise<string> {
+    let output = "";
+    const { promise, resolve, reject } = Promise.withResolvers<string>();
+
+    // 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<void>();
     const terminal = new Bun.Terminal({