fix: allow lifecycle hooks to accept options as second parameter (#24039)

## Summary

Fixes #23133

This PR fixes a bug where lifecycle hooks (`beforeAll`, `beforeEach`,
`afterAll`, `afterEach`) would throw an error when called with a
function and options object:

```typescript
beforeAll(() => {
  console.log("beforeAll")
}, { timeout: 10_000 })
```

Previously, this would throw: `error: beforeAll() expects a function as
the second argument`

## Root Cause

The issue was in `ScopeFunctions.parseArguments()` at
`src/bun.js/test/ScopeFunctions.zig:342`. When parsing two arguments, it
always treated them as `(description, callback)` instead of checking if
they could be `(callback, options)`.

## Solution

Updated the two-argument parsing logic to check if the first argument is
a function and the second is not a function. In that case, treat them as
`(callback, options)` instead of `(description, callback)`.

## Changes

- Modified `src/bun.js/test/ScopeFunctions.zig` to handle `(callback,
options)` case
- Added regression test at `test/regression/issue/23133.test.ts`

## Testing

✅ Verified the fix works with the reproduction case from the issue
✅ Added comprehensive regression test covering all lifecycle hooks with
both object and numeric timeout options
✅ All existing jest-hooks tests still pass
✅ Test fails with `USE_SYSTEM_BUN=1` and passes with the fixed build

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: pfg <pfg@pfg.pw>

src/bun.js/test/ScopeFunctions.zig

@@ -296,6 +296,7 @@ const ParseArgumentsResult = struct {
     }
 };
 pub const CallbackMode = enum { require, allow };
+pub const FunctionKind = enum { test_or_describe, hook };
 
 fn getDescription(gpa: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, description: jsc.JSValue, signature: Signature) bun.JSError![]const u8 {
     if (description == .zero) {
@@ -329,7 +330,7 @@ fn getDescription(gpa: std.mem.Allocator, globalThis: *jsc.JSGlobalObject, descr
     return globalThis.throwPretty("{s}() expects first argument to be a named class, named function, number, or string", .{signature});
 }
 
-pub fn parseArguments(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, signature: Signature, gpa: std.mem.Allocator, cfg: struct { callback: CallbackMode }) bun.JSError!ParseArgumentsResult {
+pub fn parseArguments(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, signature: Signature, gpa: std.mem.Allocator, cfg: struct { callback: CallbackMode, kind: FunctionKind = .test_or_describe }) bun.JSError!ParseArgumentsResult {
     var a1, var a2, var a3 = callframe.argumentsAsArray(3);
 
     const len: enum { three, two, one, zero } = if (!a3.isUndefinedOrNull()) .three else if (!a2.isUndefinedOrNull()) .two else if (!a1.isUndefinedOrNull()) .one else .zero;
@@ -338,8 +339,9 @@ pub fn parseArguments(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame
         // description, callback(fn), options(!fn)
         // description, options(!fn), callback(fn)
         .three => if (a2.isFunction()) .{ .description = a1, .callback = a2, .options = a3 } else .{ .description = a1, .callback = a3, .options = a2 },
+        // callback(fn), options(!fn)
         // description, callback(fn)
-        .two => .{ .description = a1, .callback = a2 },
+        .two => if (a1.isFunction() and !a2.isFunction()) .{ .callback = a1, .options = a2 } else .{ .description = a1, .callback = a2 },
         // description
         // callback(fn)
         .one => if (a1.isFunction()) .{ .callback = a1 } else .{ .description = a1 },
@@ -352,7 +354,8 @@ pub fn parseArguments(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame
     } else if (callback.isFunction()) blk: {
         break :blk callback.withAsyncContextIfNeeded(globalThis);
     } else {
-        return globalThis.throw("{s} expects a function as the second argument", .{signature});
+        const ordinal = if (cfg.kind == .hook) "first" else "second";
+        return globalThis.throw("{s} expects a function as the {s} argument", .{ signature, ordinal });
     };
 
     var result: ParseArgumentsResult = .{

src/bun.js/test/bun_test.zig

@@ -44,7 +44,7 @@ pub const js_fns = struct {
                 defer group.end();
                 errdefer group.log("ended in error", .{});
 
-                var args = try ScopeFunctions.parseArguments(globalThis, callFrame, .{ .str = @tagName(tag) ++ "()" }, bun.default_allocator, .{ .callback = .require });
+                var args = try ScopeFunctions.parseArguments(globalThis, callFrame, .{ .str = @tagName(tag) ++ "()" }, bun.default_allocator, .{ .callback = .require, .kind = .hook });
                 defer args.deinit(bun.default_allocator);
 
                 const has_done_parameter = if (args.callback) |callback| try callback.getLength(globalThis) > 0 else false;

test/regression/issue/23133.test.ts

@@ -0,0 +1,54 @@
+// https://github.com/oven-sh/bun/issues/23133
+// Passing HookOptions to lifecycle hooks should work
+import { afterAll, afterEach, beforeAll, beforeEach, expect, test } from "bun:test";
+
+const logs: string[] = [];
+
+// Test beforeAll with object timeout option
+beforeAll(
+  () => {
+    logs.push("beforeAll with object timeout");
+  },
+  { timeout: 10_000 },
+);
+
+// Test beforeAll with numeric timeout option
+beforeAll(() => {
+  logs.push("beforeAll with numeric timeout");
+}, 5000);
+
+// Test beforeEach with timeout option
+beforeEach(
+  () => {
+    logs.push("beforeEach");
+  },
+  { timeout: 10_000 },
+);
+
+// Test afterEach with timeout option
+afterEach(
+  () => {
+    logs.push("afterEach");
+  },
+  { timeout: 10_000 },
+);
+
+// Test afterAll with timeout option
+afterAll(
+  () => {
+    logs.push("afterAll");
+  },
+  { timeout: 10_000 },
+);
+
+test("lifecycle hooks accept timeout options", () => {
+  expect(logs).toContain("beforeAll with object timeout");
+  expect(logs).toContain("beforeAll with numeric timeout");
+  expect(logs).toContain("beforeEach");
+});
+
+test("beforeEach runs before each test", () => {
+  // beforeEach should have run twice now (once for each test)
+  const beforeEachCount = logs.filter(l => l === "beforeEach").length;
+  expect(beforeEachCount).toBe(2);
+});