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

Merged
merged 16 commits into from
Feb 3, 2025

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 87.77778% with 11 lines in your changes missing coverage. Please review.

Project coverage is 54.68%. Comparing base (6ddc2b4) to head (690770b).
Report is 360 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/job.rs 68.75% 10 Missing ⚠️
core/runtime/src/interval.rs 98.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4130      +/-   ##
==========================================
+ Coverage   47.24%   54.68%   +7.44%     
==========================================
  Files         476      492      +16     
  Lines       46892    49377    +2485     
==========================================
+ Hits        22154    27002    +4848     
+ Misses      24738    22375    -2363     

☔ 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
Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Looking great overall! Nice job!

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.

Looks really nice!

@jedel1043 jedel1043 added this pull request to the merge queue Feb 3, 2025
@jedel1043 jedel1043 removed this pull request from the merge queue due to the queue being cleared Feb 3, 2025
@hansl hansl enabled auto-merge February 3, 2025 22:20
@hansl hansl added this pull request to the merge queue Feb 3, 2025
Merged via the queue into boa-dev:main with commit 1e71c6d Feb 3, 2025
14 checks passed
@hansl hansl deleted the set-timeout branch February 3, 2025 22:48
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.

3 participants