Skip to content
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.

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

setTimeout, setInterval and clearInterval (and the same clearTimeout) implementations #4130

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

hansl
Copy link
Contributor

@hansl hansl commented Jan 13, 2025

This adds non-async implementations of the setTimeout/setInterval API functions. They are by default registered in the context when registering all APIs from boa_runtime.

This adds a new type of jobs that implement HostEnqueueTimeoutJob (see https://tc39.es/ecma262/#sec-hostenqueuetimeoutjob).

This requires #4141 to be merged first.

@hansl hansl requested review from jedel1043 and a team January 13, 2025 23:34
@hansl hansl marked this pull request as draft January 13, 2025 23:41
hansl added 2 commits January 22, 2025 11:06
…imeout`) implementations

This adds non-async implementations of the `setTimeout`/`setInterval` API functions. They are by default registered in the context when registering all APIs from `boa_runtime`.
@hansl hansl marked this pull request as ready for review January 22, 2025 23:32
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Really nice implementation! I have some opinions on using SystemTime, but aside from that everything else looks great!

core/engine/src/job.rs Outdated Show resolved Hide resolved
core/engine/src/job.rs Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 79.59184% with 30 lines in your changes missing coverage. Please review.

Project coverage is 53.69%. Comparing base (6ddc2b4) to head (356a738).
Report is 356 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/context/time.rs 68.29% 13 Missing ⚠️
core/engine/src/job.rs 68.75% 10 Missing ⚠️
core/engine/src/object/builtins/jsdate.rs 0.00% 3 Missing ⚠️
core/engine/src/context/mod.rs 75.00% 2 Missing ⚠️
core/engine/src/builtins/date/mod.rs 80.00% 1 Missing ⚠️
core/runtime/src/interval.rs 98.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4130      +/-   ##
==========================================
+ Coverage   47.24%   53.69%   +6.44%     
==========================================
  Files         476      490      +14     
  Lines       46892    49010    +2118     
==========================================
+ Hits        22154    26314    +4160     
+ Misses      24738    22696    -2042     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hansl hansl requested review from jedel1043 and a team January 27, 2025 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants