[css-view-transitions-1] Clarify timing of updateCallbackDone

[noamr]

Currently updateCallbackDone is a reaction to the provided update callback (in startViewTransition(update)),
and activating the view transition is a reaction to updateCallbackDone.

This makes it so that author-provided updateCallbackDone reactions are usually called before capturing the new state, but sometimes after. This can create a confusion and subtle bugs.

I found that updateCallbackDone is most useful in its usual behavior, when all the author reactions are flushed and only then the new state is captured - this allows using this callback as a way to add new state changes on top of whatever is in the update callback

Proposing to slightly change the wording so that it's clear that there's a microtask checkpoint between resolving updateCallbackDone and activating the view transition (capturing the new state).

[jakearchibald]

It's worth checking with @domenic or @annevk on this. Adding microtask checkpoints to solve race conditions is usually a bad smell.

In userland, promises don't shuffle the order of reaction callbacks, or provide a way to do this.

Could you document some of the bugs you're seeing as a result of the current behaviour? There could be another solution.

[noamr]

It's worth checking with @domenic or @annevk on this. Adding microtask checkpoints to solve race conditions is usually a bad smell.

Agreed, would love to find a more elegant way around this if this is indeed a spec bug.

In userland, promises don't shuffle the order of reaction callbacks, or provide a way to do this.

Could you document some of the bugs you're seeing as a result of the current behaviour? There could be another solution.

The context is https://bugs.chromium.org/p/chromium/issues/detail?id=1513887
I've noticed that when running startViewTransition inside a navigate event, I can't update the new state inside an updateCallbackDone callback, my promise reactions are handled after the state is already captured, wasn't sure if this is an implementation or a spec thing.

[jakearchibald]

From the bug:

Changing the DOM in ViewTransition.updateCallbackDone should be equivalent to changing it in the callback itself.

Should those things be considered equivalent? Doing something after it's done shouldn't be equivalent to doing something before it's done. One of the reasons we have an updateCallback rather than a promise is that we don't want simple promise behaviour here.

[noamr]

From the bug:

Changing the DOM in ViewTransition.updateCallbackDone should be equivalent to changing it in the callback itself.

Should those things be considered equivalent? Doing something after it's done shouldn't be equivalent to doing something before it's done. One of the reasons we have an updateCallback rather than a promise is that we don't want simple promise behaviour here.

That's fair, but then it should be consistent and not racy.
Right now changing the DOM in updateCallbackDone is sometimes equivalent to changing it in the update callback, which is worse than it never being equivalent.

If the author wants a promise that always resolves after everything is captured, transition.ready.catch() or some such already works, no?

[jakearchibald]

If this is worth fixing, it seems like the right way is to make sure the reaction callbacks in https://drafts.csswg.org/css-view-transitions-1/#setup-view-transition-algorithm (5.3) are added as soon as the update callback done promise is created. That way the spec's reaction callback will always be first.

[noamr]

You mean fixing it so that vt.updateCallbackDone would be syntactic sugar to vt.ready.catch()? That would be a bit of a waste... actually since rendering is suppressed anyway, perhaps we should queue a task to activate the transition rather than do it as a direct reaction to the microtask.

It's perhaps worth a discussion, I think the purpose of updateCallbackDone is unclear, seems like mostly a thing for progressive enhancement in case the transition doesn't fire. When using it I found that it was often either racy or redundant, and what I actually needed it a way to extend the update callback after the transition was created.

[jakearchibald]

You mean fixing it so that vt.updateCallbackDone would be syntactic sugar to vt.ready.catch()? That would be a bit of a waste

No, because updateCallbackDone rejects if the promise returned from the update callback is a rejection, whereas vt.ready.catch() never rejects.

It's perhaps worth a discussion, I think the purpose of updateCallbackDone is unclear,

Does the example here make sense? https://developer.chrome.com/docs/web-platform/view-transitions/#transitions_as_an_enhancement

Also see the docs here https://drafts.csswg.org/css-view-transitions-1/#ref-for-dom-viewtransition-updatecallbackdone%E2%91%A0

what I actually needed it a way to extend the update callback after the transition was created

It doesn't do that, and that isn't what it's designed for. If you need to extend the update callback, you should be adding more to the update callback. That's why it's a callback rather than a promise, else we lose the ability to catch errors etc.

[jakearchibald]

In case the above doesn't explain it well, here's another description:

updateCallbackDone resolve when the update callback is done. If the update callback fails (as in, the callback returns a rejected promise), updateCallbackDone rejects. This is for cases where you don't care about the success/failure of the transition, you just care about the success/failure of the DOM change.

For example, when intercepting a navigation using the navigation API, the handler returns a promise relating to the success/failure of the navigation. A failed transition isn't a failed navigation, so if you're wrapping your DOM change in startViewTransition, then updateCallbackDone gives you information pertinent to the navigation API.

[noamr]

OK, proposing to resolve on making the order of events consistent:

• React to the user provided callback:
  • Activate the transition (ie capture the new state if everything is ok)
  • Only then resolve the transition's updateCallbackDone promise.

[khushalsagar]

^ The goal of the above is to ensure capturing new state deterministically happens before updateCallbackDone is resolved. If so, that SGTM.

[noamr]

Apparently this is already what's implemented in Chromium, so I'll update the spec to make it clearer.