Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 27 additions & 15 deletions packages/react-native/ReactCommon/react/runtime/TimerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,26 @@

namespace facebook::react {

double coerceNumberTimeout(jsi::Runtime& rt, const jsi::Value& timeout) {
double delay = 0.0;

// fast-path
if (timeout.isNumber()) {
delay = timeout.getNumber();
}
else {
// perform number coercion for timeout to be web spec compliant
auto numberCtor = rt.global().getPropertyAsObject(rt, "Number");
auto delayNumberObject = numberCtor.getFunction(rt).callAsConstructor(rt, timeout).getObject(rt);
auto delayNumericValue = delayNumberObject.getPropertyAsFunction(rt, "valueOf").callWithThis(rt, delayNumberObject);

delay = delayNumericValue.isNumber() ? delayNumericValue.getNumber() : 0;
}

delay = std::isnan(delay) ? 0.0 : std::max(0.0, delay);
return delay;
}

namespace {
inline const char* getTimerSourceName(TimerSource source) {
switch (source) {
Expand Down Expand Up @@ -221,9 +241,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 setImmediate must be a function.");
// Do not throw any error to match web spec
return timerIndex_++;
}
auto callback = args[0].getObject(rt).getFunction(rt);

Expand Down Expand Up @@ -276,18 +295,12 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
}

if (!args[0].isObject() || !args[0].asObject(rt).isFunction(rt)) {
throw jsi::JSError(
rt, "The first argument to setTimeout must be a function.");
// Do not throw any error to match web spec
return timerIndex_++;
}
auto callback = args[0].getObject(rt).getFunction(rt);

if (count > 1 && !args[1].isNumber() && !args[1].isUndefined()) {
throw jsi::JSError(
rt,
"The second argument to setTimeout must be a number or undefined.");
}
auto delay =
count > 1 && args[1].isNumber() ? args[1].getNumber() : 0;
auto callback = args[0].getObject(rt).getFunction(rt);
auto delay = count > 1 ? coerceNumberTimeout(rt, jsi::Value {rt, args[1] }) : 0.0;

// Package up the remaining argument values into one place.
std::vector<jsi::Value> moreArgs;
Expand Down Expand Up @@ -344,8 +357,7 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
rt, "The first argument to setInterval must be a function.");
}
auto callback = args[0].getObject(rt).getFunction(rt);
auto delay =
count > 1 && args[1].isNumber() ? args[1].getNumber() : 0;
auto delay = count > 1 ? coerceNumberTimeout(rt, jsi::Value { rt, args[1] }) : 0.0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no need to create a new jsi::Value here

Suggested change
auto delay = count > 1 ? coerceNumberTimeout(rt, jsi::Value { rt, args[1] }) : 0.0;
auto delay = count > 1 ? coerceNumberTimeout(rt, args[1]) : 0.0;


// Package up the remaining argument values into one place.
std::vector<jsi::Value> moreArgs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,12 @@ TEST_F(ReactInstanceTest, testSetImmediateWithInvalidArgs) {
EXPECT_EQ(
getErrorMessage("setImmediate();"),
"setImmediate must be called with at least one argument (a function to call)");
EXPECT_EQ(
getErrorMessage("setImmediate('invalid');"),
"The first argument to setImmediate must be a function.");
EXPECT_EQ(
getErrorMessage("setImmediate({});"),
"The first argument to setImmediate must be a function.");

eval("setImmediate('invalid');");
expectNoError();

eval("setImmediate({});");
expectNoError();
}

TEST_F(ReactInstanceTest, testClearImmediate) {
Expand Down Expand Up @@ -420,12 +420,12 @@ TEST_F(ReactInstanceTest, testSetTimeoutWithInvalidArgs) {
EXPECT_EQ(
getErrorMessage("setTimeout();"),
"setTimeout must be called with at least one argument (the function to call).");
EXPECT_EQ(
getErrorMessage("setTimeout('invalid');"),
"The first argument to setTimeout must be a function.");
EXPECT_EQ(
getErrorMessage("setTimeout(() => {}, 'invalid');"),
"The second argument to setTimeout must be a number or undefined.");

eval("setTimeout('invalid');");
expectNoError();

eval("setTimeout(() => {}, 'invalid');");
expectNoError();
}

TEST_F(ReactInstanceTest, testClearTimeout) {
Expand Down
4 changes: 3 additions & 1 deletion packages/rn-tester/js/examples/Timer/TimerExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class RequestIdleCallbackTester extends React.Component<
}

type TimerTesterProps = $ReadOnly<{|
dt?: number,
dt?: any,
type: string,
|}>;

Expand Down Expand Up @@ -328,6 +328,8 @@ exports.examples = [
<TimerTester type="setTimeout" dt={0} />
<TimerTester type="setTimeout" dt={1} />
<TimerTester type="setTimeout" dt={100} />
<TimerTester type="setTimeout" dt={{valueOf: () => 200}} />
<TimerTester type="setTimeout" dt={'500'} />
</View>
);
},
Expand Down