Skip to content

fix: Align TimerManager sequential ids and function error handling with web standard #51500

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
wants to merge 6 commits into from
Closed

fix: Align TimerManager sequential ids and function error handling with web standard #51500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
deab62e
Update TimerManager timerIndex_ to start at 1 instead of 0
kitten May 21, 2025
a3d0ccb
Return 0 instead of incremented timer id
kitten May 21, 2025
935a163
Remove invalid thrown error in setInterval and instead return 0
kitten May 21, 2025
05d6d7b
test: Expect 0 as return value for invalid setInterval handler argument
kitten May 21, 2025
54098aa
test: Expect 0 as return value for invalid setTimeout handler argument
kitten May 21, 2025
800885d
test: Timer ID should start at 1 not 0
kitten May 21, 2025
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
8 changes: 4 additions & 4 deletions packages/react-native/ReactCommon/react/runtime/TimerManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
}

if (!args[0].isObject() || !args[0].asObject(rt).isFunction(rt)) {
// Do not throw any error to match web spec
return timerIndex_++;
// Do not throw any error to match web spec; instead return 0, an invalid timer id
return 0;
}

auto callback = args[0].getObject(rt).getFunction(rt);
Expand Down Expand Up @@ -300,8 +300,8 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
}

if (!args[0].isObject() || !args[0].asObject(rt).isFunction(rt)) {
throw jsi::JSError(
rt, "The first argument to setInterval must be a function.");
// Do not throw any error to match web spec; instead return 0, an invalid timer id
return 0;
}
auto callback = args[0].getObject(rt).getFunction(rt);
auto delay = count > 1
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native/ReactCommon/react/runtime/TimerManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class TimerManager {

// Each timeout that is registered on this queue gets a sequential id. This
// is the global count from which those are assigned.
TimerHandle timerIndex_{0};
// As per WHATWG HTML 8.6.1 (Timers) ids must be greater than zero, i.e. start at 1
TimerHandle timerIndex_{1};

// The React Native microtask queue is used to back public APIs including
// `queueMicrotask`, `clearImmediate`, and `setImmediate` (which is used by
Expand Down
14 changes: 9 additions & 5 deletions packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,9 @@ TEST_F(ReactInstanceTest, testSetTimeoutWithoutDelay) {
EXPECT_CALL(
*mockRegistry_,
createTimer(_, 0)); // If delay is not provided, it should use 0
eval("setTimeout(() => {});");
auto val = eval("setTimeout(() => {});");
expectNoError();
EXPECT_EQ(val.asNumber(), 1); // First timer id should start at 1
}

TEST_F(ReactInstanceTest, testSetTimeoutWithPassThroughArgs) {
Expand Down Expand Up @@ -299,8 +301,9 @@ TEST_F(ReactInstanceTest, testSetTimeoutWithInvalidArgs) {
getErrorMessage("setTimeout();"),
"setTimeout must be called with at least one argument (the function to call).");

eval("setTimeout('invalid');");
auto val = eval("setTimeout('invalid')");
expectNoError();
EXPECT_EQ(val.asNumber(), 0);

eval("setTimeout(() => {}, 'invalid');");
expectNoError();
Expand Down Expand Up @@ -417,9 +420,10 @@ TEST_F(ReactInstanceTest, testSetIntervalWithInvalidArgs) {
EXPECT_EQ(
getErrorMessage("setInterval();"),
"setInterval must be called with at least one argument (the function to call).");
EXPECT_EQ(
getErrorMessage("setInterval('invalid', 100);"),
"The first argument to setInterval must be a function.");

auto val = eval("setInterval('invalid', 100)");
expectNoError();
EXPECT_EQ(val.asNumber(), 0);
}

TEST_F(ReactInstanceTest, testClearInterval) {
Expand Down
Loading