cannot pass extra arguments to setTimeout() & friends

[alexlamsl]

What version of Bun is running?

--canary

What platform is your computer?

Linux 5.15.0-56-generic x86_64 x86_64

What steps can reproduce the bug?

$ cat test.js
setTimeout(function() {
  console.log(arguments.length, ...arguments);
}, 0, "arg1", "arg2");

What is the expected behavior?

$ node test.js
2 arg1 arg2

What do you see instead?

$ bun test.js
0

Additional information

Same goes for setImmediate():

$ cat test-immediate.js
setImmediate(function() {
  console.log(arguments.length, ...arguments);
}, "arg1", "arg2");

$ node test-immediate.js
2 arg1 arg2

$ bun test-immediate.js
0

whereas process.nextTick() differs... differently 😅

$ cat test-tick.js
process.nextTick(function() {
  console.log(arguments.length, ...arguments);
}, "arg1", "arg2");

$ node test-tick.js
2 arg1 arg2

$ bun test-tick.js
3 arg1 arg2 undefined

[Jarred-Sumner]

i guess we just forgot to implement this

[Jarred-Sumner]

I wrote the code for this and haven't pushed it up yet I should've said something

[alexlamsl]

No worries − it's good exercise for me either which way 😉

[Jarred-Sumner]

Overall though the code is really good given I didn't share any context on the C++ code.

The downside to JSC::BoundFunction is it's an extra wrapper function which adds ~10ns to call overhead.

A different approach is copying the arguments into a JSC::Array and then adding an extra argument to the set.* functions.

A more optimal approach is creating a custom subclass like JSTimerCallback which has a JSFunction and maybe 2 additional mutable JSC::WriteBarrier<JSC::Unknown> along with a length. Then we hold a strong reference to that and call it. I didn't implement this version. I went with copying into an array (using the faster tryCreateUninitializedRestricted version) but only if there are extra arguments and then adding a separate JSC::Strong

[alexlamsl]

Do you want me to separate out the wiptest test updates from #1635, or are you good with just copying them over?

[Jarred-Sumner]

Please keep the tests as the PR and I'll merge that. Part of why I didn't push yet was lack of tests