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

Replace transitionWhile()/canTransition with intercept()/canIntercept #235

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Jun 2, 2022

Fixes #230. Mini-explainer:

The "navigate" event provided by the navigation API has a method on its NavigateEvent instance, event.transitionWhile(promise, options). This method would intercept the navigation, convert it into a same-document navigation, and then use the provided promise to signal to the browser and to other parts of the web application that we were in a transitional phase while the new navigation finishes up.

However, the signature of transitionWhile(promise) worked poorly in practice. The most important reason is the common coding pattern of:

event.transitionWhile((async () => {
  doSyncStuff();
  await doAsyncStuff();
})());

i.e., the common pattern of generating a promise via an immediately-invoked async function. The problem with this pattern is that it's equivalent to

doSyncStuff();
event.transitionWhile((async () => {
  await doAsyncStuff();
})());

This means some portion of your "navigation handling code" would run, before the navigate event had finished firing and the URL/entry had been updated.

We know of at least two bad examples of this:

Our solution is to replace transitionWhile(promise, options) with intercept({ handler, ...options }). Essentially, we are replacing the idea of you passing a promise---which people would often generate with an async function---with the idea of you passing an async function directly. Then, the user agent can be sure to schedule the entirety of the async function after the navigate event has finished firing, and the URL/entry updated. This avoids the above problems.

This particular API shape has other minor benefits:

  • Code such as

    event.transitionWhile((async () => {
      // ... many lines here ...
    })(), { focusReset: "manual" });

    hid some fairly-important information (the focusReset: "manual") down at the bottom. With the new structure, we can instead write

    event.intercept({
      focusReset: "manual",
      async handler() {
        // ... many lines here ...
      }
    });
  • It is common in applications that are just starting to use the navigation API, and are not yet ready to centralize their routing logic into the "navigate" event handler, to simply want to intercept the navigation and convert it into a navigation-API-handled one, but leave the rest of their application as the one that reacts to this navigation. Previously this would require

    event.transitionWhile(Promise.resolve());

    which felt like a hack. ("I want to transition, while an already-fulfilled promise is outstanding"!?) Now it is simply

    event.intercept();

    which communicates the intent better.

  • The immediately-invoked async function pattern was ugly. So ugly we were contemplating JavaScript language and Web IDL-level changes to get rid of it. See Maybe allow respondWith to take a promise-returning function? #40 and Allow functions to be used as promise inputs whatwg/webidl#1020. These changes are now not necessary, at least for our API.

  • This change slightly reduces (but does not fully remove) the confusion between the navigation API's notion of a navigation transition, and the in-progress shared element transitions spec's idea of a navigation transition. See Naming clash with navigation API view-transitions#143. The reason it does not eliminate it is that navigation.transition still exists. (We may settle this by renaming navigation.transition; see transitionWhile doesn't quite work for scroll restoration #230 (comment).)


Preview | Diff

@domenic domenic requested a review from natechapin June 2, 2022 22:56
domenic added a commit that referenced this pull request Jun 6, 2022
Previously, we were setting an entry's serialized state as part of the end of the navigate() call. This will work less well after #235; there, we need to set the state right after the commit, but before any handlers are called. So most of this patch is just a for-now-editorial rearrangement, to set the serialized state in a different place in the spec (but observably the same given the current API).

One part of this includes rearranging things so that if options["state"] is not given to reload() or navigate(), we now pass through the serialization of undefined, instead of passing through null. This allows null to be used as a sentinel for traverse cases. This change is also not observable, since getState() would turn null into undefined.
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 8, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
insetad of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1183545
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 13, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
insetad of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 13, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
insetad of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Copy link
Collaborator

@natechapin natechapin left a comment

Choose a reason for hiding this comment

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

Looks good! Just one optional nitpick.

spec.bs Outdated Show resolved Hide resolved
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 14, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
domenic added a commit that referenced this pull request Jun 15, 2022
Previously, we were setting an entry's serialized state as part of the end of the navigate() call. This will work less well after #235; there, we need to set the state right after the commit, but before any handlers are called. So most of this patch is just a for-now-editorial rearrangement, to set the serialized state in a different place in the spec (but observably the same given the current API).

One part of this includes rearranging things so that if options["state"] is not given to reload() or navigate(), we now pass through the serialization of undefined, instead of passing through null. This allows null to be used as a sentinel for traverse cases. This change is also not observable, since getState() would turn null into undefined.
@domenic domenic merged commit da4277f into main Jun 15, 2022
@domenic domenic deleted the replace-intercept branch June 15, 2022 17:02
@jakearchibald
Copy link

I know I caused a lot of trouble by whinging about this late in the process, but I'm really grateful that this change happened. I'm giving a talk on this later, and this API makes it so much easier to explain (especially as I'm talking about page transitions in the same talk).

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 17, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 17, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 22, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 23, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 23, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 11, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 11, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 12, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 19, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 20, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 20, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 20, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 22, 2022
…navigateEvent.canIntercept, a=testonly

Automatic update from web-platform-tests
Implement navigateEvent.intercept() and navigateEvent.canIntercept

Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}

--

wpt-commits: 5f0779f40dbb30349778db64db04754df8c4a4ef
wpt-pr: 34353
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 27, 2022
…navigateEvent.canIntercept, a=testonly

Automatic update from web-platform-tests
Implement navigateEvent.intercept() and navigateEvent.canIntercept

Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}

--

wpt-commits: 5f0779f40dbb30349778db64db04754df8c4a4ef
wpt-pr: 34353
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Follows WICG/navigation-api#235

intercept() works very similarly to transitionWhile(), except that
instead of taking a mandatory Promise, it takes an optional handler
function. If a function is provided and it returns a promise,
navigation finish will be delayed until the Promise resolve, just as
transitionWhile() delays navigation finish for its Promise.

canIntercept is identical to canTransition.

Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/jyWqjAEv5LU

Bug: 1336000
Change-Id: I94edd7fdc727080594f16fe4511cb7c302d88941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688177
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1026325}
NOKEYCHECK=True
GitOrigin-RevId: d53817e29d5e857782ff8d977dbc3b89eb53831b
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.

transitionWhile doesn't quite work for scroll restoration
3 participants