New principle: A Promise represents completion or a value, not a callback (#342)

[jan-ivar]

Fixes #342.

---

Preview | Diff

[martinthomson]

This LGTM. Time to ask the rest of the TAG to review.

[jan-ivar]

Thanks @martinthomson!

I appear to be unable to add reviewers in this repo, so hopefully there's a process for that?

[LeaVerou]

Overall this seems a bit esoteric, and I agree with @jyasskin that the title as stated is not always true. I think the examples need some work to establish what should or shouldn't be done, otherwise this principle only makes sense to those who already understand it.

[jan-ivar]

Feedback incorporated. Thanks!

[jan-ivar]

We could also shorten it further to: "A Promise represents completion, not a callback" — thoughts?

[martinthomson]

I like this. I think that the one thing that might be added

If an API depends on setting up temporary conditions then invoking the caller,
that is a good reason to use a callback rather than a promise.

[jan-ivar]

@LeaVerou does this look better?

[jyasskin]

Sorry for taking a long time to review this. I ... rewrote a big chunk of it .... What do you think?

[jyasskin]

I think this section belongs next to https://pr-preview.s3.amazonaws.com/jan-ivar/design-principles/pull/496.html#promises, not just at the end of the parent section.

[jan-ivar]

Won't that renumber § 6.11 through § 6.14?

[jyasskin]

Yes. I'm double-checking with other TAG folks, but I think it's a very bad idea for anyone to refer to sections of a living document by number, and we shouldn't be constrained to preserve those numbers.

[jyasskin]

I think this one can actually be "Never", because of the composition pitfall you're describing. Hm, except that you have "Never limit the viability..." down below, implying that you were trying for something more general here. How about:

Suggested change

	Avoid requiring that something needs to happen synchronously within
	a promise resolution or rejection callback.
	When a Promise settles (is fulfilled or rejected),
	that tells the developer that
	certain facts have become true of the execution environment.
	Developers tend to assume that these facts have "inertia":
	that they will remain true until something acts on them.
	When the platform automatically changes some of these facts,
	that risks introducing bugs.
	The platform must not change these facts between microtasks.
	Always wait at least until a new task runs.

[martinthomson]

You seem to have hit on something useful that is at the core of this:

• The state of the execution environment, as presented to the developer, cannot change except when executing a new task.

It's OK for the information presented to be a view of the state of things that might subsequently (or even immediately) be invalidated. If the API relates to something that is acting outside of the sandbox, like a camera or another application, then there is no way to be certain that information presented about that thing is correct. The best you can do is send it messages and see what happens. But things that comprise the environment are there to serve the developer's needs.

The whole Promise resolution thing is secondary to all of that. Maybe there is a different framing we could use that highlights that.

[jan-ivar]

Thanks for the review and text!

The state of the execution environment, as presented to the developer, cannot change except when executing a new task.

I'm not sure we can say the environment is immutable. E.g. JS can call track.stop() and synchronously observe track.readyState changed from "live" to "ended". Only in-parallel changes seem problematic.

Also, apart from the busy-wait timer (which is more like ~1 second and a mitigation really), I don't think it's accurate to describe an overly strict and synchronous API contract as an in-parallel change to the environment.

The platform already seems to contain examples of synchronous API contracts, like preventDefault() that don't seem to violate § 5.2. Preserve run-to-completion semantics.

I think the anti-pattern is applying such synchronous API contracts to a promise callback.

[jyasskin]

Right, the environment can certainly change in response to the developer actively doing something. Thanks for pointing out "Preserve run-to-completion semantics". Maybe the bit about not changing things between microtasks should actually be part of that section. It could use some tweaking anyway because "while a JavaScript event loop is running" includes the time between tasks when we want the platform to change things.

But then what's left for this section to say? There is something interesting about the way that a Promise says "a new state has started", while a callback says "a state is present during this function invocation", but I'm not sure exactly how to express it. setFocusBehavior() is going to violate the rule regardless ... and maybe that's a sign that it should actually be a callback in the DisplayMediaStreamOptions, and not have special permission to run for a short time after the Promise resolves?

[jan-ivar]

A singular underlying principle is encouraging, suggesting we're on the right track, or at least being consistent.

But practically speaking, might it still deserve two guidelines if the patterns to avoid are distinct enough?

Anti-pattern 1:

• rationale: app must do something right away or not at all; a microtask window seems shorter than a task
• symptom: breaks the async-functions-are-wrappable invariant

Anti-pattern 2:

• rationale: mitigate busy-waiting exploits; a 1 second timer deadline won't bother well-behaved apps
• symptom: breaks malicious or already broken apps as a secondary action-at-a-distance symptom

Reasons to treat them separately:

• the first one seems more serious than the second
• the second one seems in the long-running scripts mitigation category; exempt from guidelines?
• setFocusBehavior() went from violating only the first to violating only the second; an improvement?

I'm concerned these distinctions might be lost if we merge/generalize the language too much.

[jyasskin]

Thinking about what this section actually means, maybe it's something like:

Suggested change

	<h3 id="allow-microtask-switches">A Promise represents completion or a value, not a callback</h3>
	<h3 id="promises-stay-fulfilled">Don't automatically undo the state represented by a Promise settling</h3>

That implies that setFocusBehavior() violates the principle, but we expect principles to sometimes be violated for good reasons. The text below gives some hints about when to expect violations of this principle, although maybe we can improve those hints...

[jan-ivar]

This sounds like a different principle. AFAIK it's not possible to unfulfill a promise, so I find this confusing. Also, if setFocusBehavior() violates it, it's not the same principle, since setFocusBehavior() is an example of the principle I had in mind applied correctly.

[jyasskin]

Given my suggestion above, some of this text becomes redundant:

Suggested change

	In general, the settled result from an asynchronous function
	should be usable at any time.
	However, if the result of your API is timing sensitive, you might need
	to restrict when it can be acted on.
	In the above example, perhaps the `platform.foo()` function
	establishes state that has real-time dependencies such that calling
	`bar()` is not viable at any future time.
	Never limit the viability of acting on a settled result to a microtask,
	as this prevents most forms of code composition.
	If possible, set a short timeout interval. Failing that, viability can
	be limited to the same task, but not a single microtask.
	<div class="example">
	One case where this came up was the [captureController.setFocusBehavior()](https://w3c.github.io/mediacapture-screen-share/#dom-capturecontroller-setfocusbehavior)
	method which controls whether a window the user selects to screen-capture
	should immediately be pushed to the front or not. It can be called as late
	as right <em>after</em> the resolution of the `getDisplayMedia()` promise. This allows
	applications to make a different decision based on the window the user chose.
	But the application must act right away or this becomes surprising to the user.
	The solution was to add a timeout on top of the requirement of it being
	called on the same task that resolves the `getDisplayMedia()` promise.
	</div>
	Occasionally, users need the state represented by a Promise to automatically move on.
	<div class="example">
	Developers need to be able to call
	<code>captureController.{{CaptureController/setFocusBehavior()}}</code>
	after the Promise returned by {{MediaDevices/getDisplayMedia()}}
	so that they can use attributes of the window the user chose
	when deciding how focus should change.
	However, users will be surprised if
	the focus change doesn't feel like an immediate effect from selecting a window,
	so {{CaptureController/setFocusBehavior()}} needs to automatically stop working
	shortly after that Promise fulfills.
	</div>
	A timeout can be a good trigger for these automatic state changes,
	especially when they're happening to aid user perception.
	If you need a faster automatic state change,
	you can [=queue a task=] to make the change.
	As mentioned above,
	never change the state established by a Promise between microtasks.

[jan-ivar]

If you need a faster automatic state change, you can [=queue a task=] to make the change.

Faster in the non-degenerate case. To clarify the example, setFocusBehavior() does both: It queues "a task to run the finalize focus decision algorithm" AND applies a deadline for that task.

IOW, the timer is only there to catch busy-waiting exploits in the callback.

[jyasskin]

Maybe:

Suggested change

	If an API depends on setting up temporary conditions then invoking the caller,
	that is a good reason to use a callback rather than a promise.
	If a feature requires the developer to only rely on some state between precise points within a task,
	that can be a good reason to use a callback rather than a promise.

[jyasskin]

Yes. I'm double-checking with other TAG folks, but I think it's a very bad idea for anyone to refer to sections of a living document by number, and we shouldn't be constrained to preserve those numbers.

[jyasskin]

Right, the environment can certainly change in response to the developer actively doing something. Thanks for pointing out "Preserve run-to-completion semantics". Maybe the bit about not changing things between microtasks should actually be part of that section. It could use some tweaking anyway because "while a JavaScript event loop is running" includes the time between tasks when we want the platform to change things.

But then what's left for this section to say? There is something interesting about the way that a Promise says "a new state has started", while a callback says "a state is present during this function invocation", but I'm not sure exactly how to express it. setFocusBehavior() is going to violate the rule regardless ... and maybe that's a sign that it should actually be a callback in the DisplayMediaStreamOptions, and not have special permission to run for a short time after the Promise resolves?

[jan-ivar]

Note I replied a few days ago in two threads above, which github isn't that great at highlighting. I'm a bit worried going too general here risks losing the advice.