CentreForDigitalHumanities / CentreForDigitalHumanities/js-timing-util

fastTimeout tests might actually run setImmediate under the hood

Open
#2 0 comments 0 reactions 0 assignees Claimed by @jgonggrijp View on GitHub
good first issue question
Dominant language
JavaScript
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

While debugging, I had a look at the bundle that Rollup generates inside the Karma test runner. It appears to wrap the test module in some code that injects the `setImmediate` function from Node.js. This is probably done by `@rollup/plugin-node-resolve`. If this is indeed the case, then the tests are not actually checking whether `fastTimeout` is implemented correctly, but whether `setImmediate` is a good alternative. This could be determined by running the tests in debug mode and setting a breakpoint in the `fastTimeout` function. If the breakpoint is never encountered, we are testing `setImmediate` instead.

A possible fix is to sophisticate the `export default` statement in `src/fastTimeout.js`. Right now, it prioritizes `setImmediate` if present. This could be changed to prioritize our own `fastTimeout` if `postMessage` is present. This might be a good idea, anyway. There should probably also be a third alternative in case neither is present; `_.defer` would work for that scenario, which is equivalent to `_.partial(setTimeout, _, 0)`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.