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.

Sign up for GitHub

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

Improve the run-to-completion principle. #536

Merged
merged 9 commits into from
Dec 17, 2024
Merged

Improve the run-to-completion principle. #536

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d892efa
Improve the run-to-completion principle.
jyasskin Dec 6, 2024
4b0c905
Warn against letting an attribute change in the middle of a statement.
jyasskin Dec 8, 2024
420d7ac
Apply Martin's suggestions.
jyasskin Dec 9, 2024
0875e30
Remove a redundant sentence of emphasis on not changing attributes in…
jyasskin Dec 12, 2024
483f437
Be more general than `while true`.
jyasskin Dec 13, 2024
088ec62
Move a positive statement to the top of the principle.
jyasskin Dec 17, 2024
4ea05e1
Remove irrelevant and incorrect claims.
jyasskin Dec 17, 2024
7a896a6
Apply changes resulting from Jan-Ivar's code review
jyasskin Dec 17, 2024
21262c9
Identify which LockManager.
jyasskin Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ spec:webidl
type:idl; text:long
type:idl; text:short
type:interface; text:double
spec:web-locks; type:interface; text:LockManager
</pre>
<pre class='ignored-specs'>
spec: css21
Expand Down Expand Up @@ -1437,34 +1438,62 @@ to have bindings in other programming languages.

<h3 id="js-rtc">Preserve run-to-completion semantics</h3>

Don't modify data accessed via JavaScript APIs
while a JavaScript <a>event loop</a> is running.

Comment on lines -1440 to -1442
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we need a positive statement for this principle.

Maybe

Propagate changes to state
that originate outside of JavaScript execution context
between tasks, not within them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, or maybe by moving up the "a task should be queued" suggestion below...

If a change to state originates outside of the JavaScript execution context,
propagate that change to JavaScript between tasks,
for example by [[html#queuing-tasks|queuing a task]],
or as part of [=update the rendering=].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How's this? I also simplified the next paragraph, and focused it on async changes instead of claiming that https://html.spec.whatwg.org/multipage/webappapis.html#killing-scripts doesn't exist.

A JavaScript Web API is generally a wrapper around
a feature implemented in a lower-level language,
such as C++ or Rust.
Unlike those languages,
when using JavaScript developers can expect
that once a piece of code begins executing,
it will continue executing until it has completed.
If a change to state originates outside of the JavaScript execution context,
propagate that change to JavaScript between tasks,
for example by [[html#queuing-tasks|queuing a task]],
or as part of [=update the rendering=].

Unlike lower-level languages
such as C++ or Rust,
JavaScript has historically acted as if
only one piece of code can execute at once.
Because of that, JavaScript authors take for granted
that the data available to a function won’t change unexpectedly
while the function is running.

So if a JavaScript Web API exposes some piece of data,
such as an object property,
the user agent must not update that data
while a JavaScript task is running.
Instead, if the underlying data changes,
<a>queue a task</a> to modify the exposed version of the data.
Changes that are not the result of developer action
and changes that are asynchronously delivered
should not happen in the middle of other JavaScript,
including between [=microtasks=].

<div class="example">
If a JavaScript task has accessed the {{NavigatorOnline/onLine|navigator.onLine}} property,
and browser's online status changes,
the property won't be updated until the next task runs.
During synchronous execution (such as a `while` loop),
and after `await`ing an already-resolved `Promise`,
developers are *unlikely* to expect things like:

* The DOM to update as a result of the HTML parser loading new content from the network
* {{HTMLImageElement/width|img.width}} to change as a result of loading image data from the network
* Buttons of a {{Gamepad}} to change state
* {{Element/scrollTop}} to change, even if scrolling can visually occur
* A synchronous method to act differently depending on asynchronous state changes.
For example, if {{LockManager}} had synchronous methods,
their behavior would depend on concurrent calls in other windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is "changes" refers not only to readable properties, but any state changes that might trip up the developer's code execution, like the observable behavior of a method they invoke. Should we add an example of that? Maybe something like:

Suggested change
* A method acting differently depending on which [=microtask=] it is called on.

Copy link
Contributor Author

@jyasskin jyasskin Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, good point that we should specifically include changes in method behavior. I think the wording you've suggested implies that folks might be tempted to identify microtasks, which they generally aren't, so I'd like to call out a particular function. I had trouble finding a synchronous method that does actually change behavior depending on queued changes, and ones that return Promises can always return in a new task, so I think I have to use a counterfactual:

Suggested change
* A synchronous method to act differently depending on asynchronous state changes.
For example, if {{LockManager}} had synchronous methods,
their behavior would depend on concurrent calls in other windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

A method acting differently depending on which [=microtask=] it is called on.

fwiw, a close example of this is requestAnimationFrame. It makes sense that it sometimes runs the callback before the next frame, and sometimes after, but it's annoying that you don't know which of these will happen. whatwg/html#10113

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I can see a similar principle that requestAnimationFrame could violate, but all of the bits of the event loop transition on task boundaries, so it's not this principle. I didn't even mis-write this part of the example, since the rendering stages don't advance asynchronously. :)

Is this maybe a new principle for the HTML section that method behavior shouldn't depend on the event loop stage? Except that's not exactly what you're looking for in that issue since you're asking for a way to detect the stage rather than a method that behaves consistently. Could this be just "design requestAnimationFrame with the benefit of hindsight" rather than a generalizable principle?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, honestly, I think my example shows that:

A method acting differently depending on which [=microtask=] it is called on.

…isn't the right terminology.

I think requestAnimationFrame is ok, it's just missing that bit that tells you in advance if it'll run before or after the next frame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I think that means https://w3ctag.github.io/design-principles/#js-rtc is basically correct, and there's nothing more for me to do on this PR. Let me know or file an issue if I have that wrong. :)

These things aren't updated by the currently running script,
so they shouldn't change during the current task.

</div>

Data can update synchronously from the result of developer action.

<div class="example">
{{ChildNode/remove()|node.remove()}} changes the DOM synchronously
and is immediately observable.
</div>

A few kinds of situations justify violating this rule:

* Observing the current time,
as in {{Date/now|Date.now()}} and {{Performance/now|performance.now()}},
although note that it's also useful to present a consistent task-wide time
as in {{AnimationTimeline/currentTime|document.timeline.currentTime}}.
jyasskin marked this conversation as resolved.
Show resolved Hide resolved
* Functions meant to help developers interrupt synchronous work,
as in the case of {{IdleDeadline/timeRemaining()|IdleDeadline.timeRemaining()}}.
* States meant to protect users from surprising UI changes,
like [=transient activation=].
jyasskin marked this conversation as resolved.
Show resolved Hide resolved
Note that {{UserActivation/isActive|navigator.userActivation.isActive}}
violates [[#attributes-like-data|the guidance that recommends a method for this case]].

<h3 id="js-gc">Don't expose garbage collection</h3>

Ensure your JavaScript Web APIs don't provide a way
Expand Down
Loading