From dbad2857eaa8861a23205ecf0480d1a49c45b59b Mon Sep 17 00:00:00 2001 From: robobun Date: Tue, 20 Jan 2026 12:51:54 -0800 Subject: [PATCH] fix(test): delete setTimeout.clock property when disabling fake timers (#26285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes the `setTimeout.clock` property not being properly deleted after `jest.useRealTimers()` is called - Previously, the property was set to `false` instead of deleted, causing `hasOwnProperty` checks to return `true` - This broke React Testing Library and other libraries that check for fake timers using `Object.prototype.hasOwnProperty.call(setTimeout, 'clock')` ## Changes - Added `JSValue.deleteProperty()` binding in Zig to call JSC's `deleteProperty()` method - Updated `setFakeTimerMarker()` in `FakeTimers.zig` to delete the `clock` property when disabling fake timers - Updated existing test in `test/regression/issue/25869.test.ts` to verify correct behavior - Added new regression test in `test/regression/issue/26284.test.ts` ## Test plan - [x] Verified new test fails with system bun (before fix) - [x] Verified new test passes with debug build (after fix) - [x] Verified existing fake timer tests still pass - [x] Verified test for issue #25869 passes with fix Fixes #26284 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot Co-authored-by: Claude Opus 4.5 --- src/bun.js/bindings/JSValue.zig | 20 ++++++++ src/bun.js/bindings/bindings.cpp | 15 ++++++ src/bun.js/test/timers/FakeTimers.zig | 13 ++++-- test/regression/issue/25869.test.ts | 10 ++-- test/regression/issue/26284.test.ts | 67 +++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 test/regression/issue/26284.test.ts diff --git a/src/bun.js/bindings/JSValue.zig b/src/bun.js/bindings/JSValue.zig index 8d911d6ecc..5bf2a1d788 100644 --- a/src/bun.js/bindings/JSValue.zig +++ b/src/bun.js/bindings/JSValue.zig @@ -329,6 +329,26 @@ pub const JSValue = enum(i64) { JSC__JSValue__put(value, global, key, result); } + extern fn JSC__JSValue__deleteProperty(target: JSValue, global: *JSGlobalObject, key: *const ZigString) bool; + /// Delete a property from an object by key. Returns true if the property was deleted. + pub fn deleteProperty(target: JSValue, global: *JSGlobalObject, key: anytype) bool { + const Key = @TypeOf(key); + if (comptime @typeInfo(Key) == .pointer) { + const Elem = @typeInfo(Key).pointer.child; + if (Elem == ZigString) { + return JSC__JSValue__deleteProperty(target, global, key); + } else if (std.meta.Elem(Key) == u8) { + return JSC__JSValue__deleteProperty(target, global, &ZigString.init(key)); + } else { + @compileError("Unsupported key type in deleteProperty(). Expected ZigString or string literal, got " ++ @typeName(Elem)); + } + } else if (comptime Key == ZigString) { + return JSC__JSValue__deleteProperty(target, global, &key); + } else { + @compileError("Unsupported key type in deleteProperty(). Expected ZigString or string literal, got " ++ @typeName(Key)); + } + } + extern "c" fn JSC__JSValue__putBunString(value: JSValue, global: *JSGlobalObject, key: *const bun.String, result: jsc.JSValue) void; fn putBunString(value: JSValue, global: *JSGlobalObject, key: *const bun.String, result: jsc.JSValue) void { if (comptime bun.Environment.isDebug) diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 1572ce13a1..90c73b4fc0 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -3781,6 +3781,21 @@ extern "C" [[ZIG_EXPORT(check_slow)]] void JSC__JSValue__putMayBeIndex(JSC::Enco RETURN_IF_EXCEPTION(scope, ); } +extern "C" bool JSC__JSValue__deleteProperty(JSC::EncodedJSValue target, JSC::JSGlobalObject* globalObject, const ZigString* key) +{ + JSC::JSValue targetValue = JSC::JSValue::decode(target); + if (!targetValue.isObject()) + return false; + + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSObject* object = targetValue.getObject(); + bool result = object->deleteProperty(globalObject, Zig::toIdentifier(*key, globalObject)); + RETURN_IF_EXCEPTION(scope, false); + return result; +} + bool JSC__JSValue__isClass(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1) { JSValue value = JSValue::decode(JSValue0); diff --git a/src/bun.js/test/timers/FakeTimers.zig b/src/bun.js/test/timers/FakeTimers.zig index 39c4d0c876..4caf05d624 100644 --- a/src/bun.js/test/timers/FakeTimers.zig +++ b/src/bun.js/test/timers/FakeTimers.zig @@ -179,13 +179,16 @@ fn errorUnlessFakeTimers(globalObject: *jsc.JSGlobalObject) bun.JSError!void { fn setFakeTimerMarker(globalObject: *jsc.JSGlobalObject, enabled: bool) void { const globalThis_value = globalObject.toJSValue(); const setTimeout_fn = (globalThis_value.getOwnTruthy(globalObject, "setTimeout") catch return) orelse return; - // Set setTimeout.clock to indicate fake timers status. // testing-library/react checks Object.hasOwnProperty.call(setTimeout, 'clock') // to detect if fake timers are enabled. - // Note: We set the property to true when enabling and leave it (or set to undefined) - // when disabling. The hasOwnProperty check will still return true after disabling, - // but this is acceptable since test environments typically reset between tests. - setTimeout_fn.put(globalObject, "clock", jsc.JSValue.jsBoolean(enabled)); + if (enabled) { + // Set setTimeout.clock = true when enabling fake timers. + setTimeout_fn.put(globalObject, "clock", .true); + } else { + // Delete the clock property when disabling fake timers. + // This ensures hasOwnProperty returns false, matching Jest/Sinon behavior. + _ = setTimeout_fn.deleteProperty(globalObject, "clock"); + } } fn useFakeTimers(globalObject: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSError!jsc.JSValue { diff --git a/test/regression/issue/25869.test.ts b/test/regression/issue/25869.test.ts index ccac351c23..3a9b651592 100644 --- a/test/regression/issue/25869.test.ts +++ b/test/regression/issue/25869.test.ts @@ -32,13 +32,13 @@ test("setTimeout.clock is set after useFakeTimers", () => { } }); -test("setTimeout.clock is set to false after useRealTimers", () => { +test("setTimeout.clock is deleted after useRealTimers", () => { jest.useFakeTimers(); jest.useRealTimers(); - // Note: The clock property remains on setTimeout but is set to false. - // This differs from Jest/Sinon which removes the property entirely. - // The value being false is sufficient for most use cases. - expect((globalThis.setTimeout as any).clock).toBe(false); + // The clock property should be deleted when disabling fake timers. + // This matches Jest/Sinon behavior and ensures hasOwnProperty returns false. + expect(Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock")).toBe(false); + expect((globalThis.setTimeout as any).clock).toBe(undefined); }); test("advanceTimersByTime(0) fires setTimeout(fn, 0) timers", async () => { diff --git a/test/regression/issue/26284.test.ts b/test/regression/issue/26284.test.ts new file mode 100644 index 0000000000..6791ab79ac --- /dev/null +++ b/test/regression/issue/26284.test.ts @@ -0,0 +1,67 @@ +// https://github.com/oven-sh/bun/issues/26284 +// After useRealTimers(), hasOwnProperty('clock') should return false +import { afterEach, expect, jest, test } from "bun:test"; + +// Simulates testing-library/react's jestFakeTimersAreEnabled() function +function jestFakeTimersAreEnabled(): boolean { + // @ts-expect-error - checking for Jest fake timers markers + if (typeof jest !== "undefined" && jest !== null) { + return ( + // @ts-expect-error - checking for mock function marker + (globalThis.setTimeout as any)._isMockFunction === true || + Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock") + ); + } + return false; +} + +// Ensure fake timers are always cleaned up after each test +afterEach(() => { + jest.useRealTimers(); +}); + +test("hasOwnProperty('clock') returns false before useFakeTimers", () => { + expect(Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock")).toBe(false); + expect(jestFakeTimersAreEnabled()).toBe(false); +}); + +test("hasOwnProperty('clock') returns true after useFakeTimers", () => { + jest.useFakeTimers(); + expect(Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock")).toBe(true); + expect((globalThis.setTimeout as any).clock).toBe(true); + expect(jestFakeTimersAreEnabled()).toBe(true); +}); + +test("hasOwnProperty('clock') returns false after useRealTimers", () => { + // First enable fake timers + jest.useFakeTimers(); + expect(Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock")).toBe(true); + + // Then disable them + jest.useRealTimers(); + + // The clock property should be deleted, not just set to false + expect(Object.prototype.hasOwnProperty.call(globalThis.setTimeout, "clock")).toBe(false); + expect((globalThis.setTimeout as any).clock).toBe(undefined); + expect(jestFakeTimersAreEnabled()).toBe(false); +}); + +test("multiple useFakeTimers/useRealTimers cycles work correctly", () => { + // Cycle 1 + jest.useFakeTimers(); + expect(jestFakeTimersAreEnabled()).toBe(true); + jest.useRealTimers(); + expect(jestFakeTimersAreEnabled()).toBe(false); + + // Cycle 2 + jest.useFakeTimers(); + expect(jestFakeTimersAreEnabled()).toBe(true); + jest.useRealTimers(); + expect(jestFakeTimersAreEnabled()).toBe(false); + + // Cycle 3 + jest.useFakeTimers(); + expect(jestFakeTimersAreEnabled()).toBe(true); + jest.useRealTimers(); + expect(jestFakeTimersAreEnabled()).toBe(false); +});