-
Notifications
You must be signed in to change notification settings - Fork 23.1k
setTimeout() on Window/Worker plus consistency fixes #42072
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
Conversation
Preview URLs (7 pages)
External URLs (3)URL:
URL:
(comment last updated: 2025-12-29 01:23:35) |
|
@pepelsbey Can you please review? |
|
@wbamberg Would you also/instead consider reviewing this. I ask because it is for TrustedTypes, and we've both been looking at all of those. |
|
I'm happy to look at this, yes, but it will take me a few days until I can find the time. |
| Notice that the first function does not create a 5-second "pause" before calling the second function. | ||
| Instead, the first function is called, but waits 5 seconds to execute. | ||
| While the first function is waiting to execute, the second function is called, and a 3-second wait is applied to the second function before it executes. | ||
| Since neither the first nor the second function's timers have completed, the third function is called and completes its execution first. | ||
| Then the second follows. | ||
| Then finally the first function is executed after its timer finally completes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it me or is this explanation very confusing? It doesn't seem to distinguish the setTimeout function and the callback argument, when it talks about "functions". Like:
Instead, the first function is called, but waits 5 seconds to execute.
Surely the first function (i.e. the first setTimeout` call) is called, and executes, and returns. It's not waiting to execute (itself). It's scheduled the callback to run after 5 seconds.
While the first function is waiting to execute, the second function is called, and a 3-second wait is applied to the second function before it executes.
What is "it" here? The second function? Like, the second setTimeout? But that has executed, it's the callback that hasn't.
And:
Since neither the first nor the second function's timers have completed, the third function is called and completes its execution first.
...implies that the third function is only called if the first two haven't completed ("since...the third function is called"), but that's completely unrelated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I didn't think it worth the effort to tidy it. Arguably in reference, and at this point in JavaScript you shouldn't even need such an example and comment on what asynchronous means.
Fixed in 5008d20
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Hamish. It is true that these are unrelated to your PR, and I would not have commented if it had not been in the diffs. But if it's in the diffs, I have to read it, and if I read it I have to comment :). And I appreciate your willingness to leave things better than you found them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're willing to add a comment with a fix suggestion I will always try to integrate it. I'm just explaining why I didn't in the first place. Excuses, excuses!
| myArray.myMethod(); // prints "zero,one,two" | ||
| myArray.myMethod(1); // prints "one" | ||
| setTimeout(myArray.myMethod, 1000); // Alerts "[object Window]" after 1 second | ||
| setTimeout(myArray.myMethod, 1500, "1"); // Alerts "undefined" after 1.5 seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So maybe I've just eaten too much tofurkey, but this seems like a very complex example to illustrate a relatively simple thing. Why can't we demonstrate using something like:
const myObject = {
log() {
console.log(`myProperty: ${this.myProperty}`)
},
myProperty: 12,
}
myObject.log();
// myProperty: 12
setTimeout(myObject.log, 1000);
// myProperty: undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. Again, did not really even look at this as orthogonal to "the point".
Anyhow, I have mostly fixed in 40a026c
I say mostly, because I used this approach to highlight the problem, and the arrow solution. However I don't understand how to integrate it with bind() (and I don't want to at this point). So the example is still complicated in the final part.
Co-authored-by: wbamberg <will@bootbonnet.ca>
Co-authored-by: wbamberg <will@bootbonnet.ca>
wbamberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thank you!
|
Thank you @wbamberg - wish you the very happiest new year. |
This updates
Window.setTimeout()andWorkerGlobalScope.setTimeout()for trusted types, following the same approach as #42042eval()example on this added in TrustedTypes: Function() constructor + eval() #42462This also did some consistency fixes across both this and the
setIntervalmethods so that they have the same name for the function param (func) and the arguments are referred to asparamrather thanargs- which is consistent with the template for web apis.There were a few layout change and improvements to the docs - these map the similar changes done previously for
setInterval()This is part of project being tracked in #41507