From 36bb3b6025eb51f6e027a76a514cc7ebb29deb10 Mon Sep 17 00:00:00 2001
From: Martin Trapp <94928215+martrapp@users.noreply.github.com>
Date: Tue, 30 Apr 2024 11:46:23 +0200
Subject: [PATCH] fix(astro): newer navigation aborts existing one (#10900)
* Detection and cancelation of previous navigations and view transitions
* typos and wording
* typos and wording
* add test for animation cancelation
* second round
* final touches
* final final touches
* Clear the most recent navigation after view transition finished
---
.changeset/curly-spoons-pull.md | 5 +
.../view-transitions/src/pages/abort.astro | 29 +++
.../view-transitions/src/pages/abort2.astro | 25 +++
packages/astro/e2e/view-transitions.test.js | 66 ++++--
packages/astro/src/transitions/events.ts | 17 +-
packages/astro/src/transitions/router.ts | 188 ++++++++++++++----
6 files changed, 266 insertions(+), 64 deletions(-)
create mode 100644 .changeset/curly-spoons-pull.md
create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro
create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro
diff --git a/.changeset/curly-spoons-pull.md b/.changeset/curly-spoons-pull.md
new file mode 100644
index 000000000000..7d0d085fe475
--- /dev/null
+++ b/.changeset/curly-spoons-pull.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Detects overlapping navigation and view transitions and automatically aborts all but the most recent one.
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro
new file mode 100644
index 000000000000..7bee46f51a6d
--- /dev/null
+++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro
@@ -0,0 +1,29 @@
+---
+import { ViewTransitions } from 'astro:transitions';
+---
+
+
+
+
+
+ Abort
+
+
+
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro
new file mode 100644
index 000000000000..f4f118db8e14
--- /dev/null
+++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro
@@ -0,0 +1,25 @@
+---
+import { ViewTransitions, fade } from 'astro:transitions';
+---
+
+
+
+
+
+ Abort
+
+
+
+
+
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
index 0913380c6377..65f136531574 100644
--- a/packages/astro/e2e/view-transitions.test.js
+++ b/packages/astro/e2e/view-transitions.test.js
@@ -1426,21 +1426,55 @@ test.describe('View Transitions', () => {
'all animations for transition:names should have been found'
).toEqual(0);
});
-});
-test('transition:persist persists selection', async ({ page, astro }) => {
- let text = '';
- page.on('console', (msg) => {
- text = msg.text();
- });
- await page.goto(astro.resolveUrl('/persist-1'));
- await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
- // go to page 2
- await page.press('input[name="name"]', 'Enter');
- await expect(page.locator('#two'), 'should have content').toHaveText('Persist 2');
- expect(text).toBe('true some cool text 5 9');
-
- await page.goBack();
- await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
- expect(text).toBe('true true');
+ test('transition:persist persists selection', async ({ page, astro }) => {
+ let text = '';
+ page.on('console', (msg) => {
+ text = msg.text();
+ });
+ await page.goto(astro.resolveUrl('/persist-1'));
+ await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
+ // go to page 2
+ await page.press('input[name="name"]', 'Enter');
+ await expect(page.locator('#two'), 'should have content').toHaveText('Persist 2');
+ expect(text).toBe('true some cool text 5 9');
+
+ await page.goBack();
+ await expect(page.locator('#one'), 'should have content').toHaveText('Persist 1');
+ expect(text).toBe('true true');
+ });
+
+ test('Navigation should be interruptible', async ({ page, astro }) => {
+ await page.goto(astro.resolveUrl('/abort'));
+ // implemented in /abort:
+ // clicks on slow loading page two
+ // after short delay clicks on fast loading page one
+ // even after some delay /two should not show up
+ await new Promise((resolve) => setTimeout(resolve, 2000)); // wait is part of the test
+ let p = page.locator('#one');
+ await expect(p, 'should have content').toHaveText('Page 1');
+ });
+
+ test('animation get canceled when view transition is interrupted', async ({ page, astro }) => {
+ let lines = [];
+ page.on('console', (msg) => {
+ msg.text().startsWith('[test]') && lines.push(msg.text());
+ });
+ await page.goto(astro.resolveUrl('/abort2'));
+ // implemented in /abort2:
+ // Navigate to self with a 10 second animation
+ // shortly after starting that, change your mind an navigate to /one
+ // check that animations got canceled
+ await new Promise((resolve) => setTimeout(resolve, 1000)); // wait is part of the test
+ let p = page.locator('#one');
+ await expect(p, 'should have content').toHaveText('Page 1');
+
+ // This test would be more important for a browser without native view transitions
+ // as those do not have automatic cancelation of transitions.
+ // For simulated view transitions, the last line would be missing as enter and exit animations
+ // don't run in parallel.
+ expect(lines.join('\n')).toBe(
+ '[test] navigate to "."\n[test] navigate to /one\n[test] cancel astroFadeOut\n[test] cancel astroFadeIn'
+ );
+ });
});
diff --git a/packages/astro/src/transitions/events.ts b/packages/astro/src/transitions/events.ts
index b3921b31f0c9..969a12139943 100644
--- a/packages/astro/src/transitions/events.ts
+++ b/packages/astro/src/transitions/events.ts
@@ -25,6 +25,7 @@ class BeforeEvent extends Event {
readonly sourceElement: Element | undefined;
readonly info: any;
newDocument: Document;
+ signal: AbortSignal;
constructor(
type: string,
@@ -35,7 +36,8 @@ class BeforeEvent extends Event {
navigationType: NavigationTypeString,
sourceElement: Element | undefined,
info: any,
- newDocument: Document
+ newDocument: Document,
+ signal: AbortSignal
) {
super(type, eventInitDict);
this.from = from;
@@ -45,6 +47,7 @@ class BeforeEvent extends Event {
this.sourceElement = sourceElement;
this.info = info;
this.newDocument = newDocument;
+ this.signal = signal;
Object.defineProperties(this, {
from: { enumerable: true },
@@ -54,6 +57,7 @@ class BeforeEvent extends Event {
sourceElement: { enumerable: true },
info: { enumerable: true },
newDocument: { enumerable: true, writable: true },
+ signal: { enumerable: true },
});
}
}
@@ -76,6 +80,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent {
sourceElement: Element | undefined,
info: any,
newDocument: Document,
+ signal: AbortSignal,
formData: FormData | undefined,
loader: (event: TransitionBeforePreparationEvent) => Promise
) {
@@ -88,7 +93,8 @@ export class TransitionBeforePreparationEvent extends BeforeEvent {
navigationType,
sourceElement,
info,
- newDocument
+ newDocument,
+ signal
);
this.formData = formData;
this.loader = loader.bind(this, this);
@@ -124,7 +130,8 @@ export class TransitionBeforeSwapEvent extends BeforeEvent {
afterPreparation.navigationType,
afterPreparation.sourceElement,
afterPreparation.info,
- afterPreparation.newDocument
+ afterPreparation.newDocument,
+ afterPreparation.signal
);
this.direction = afterPreparation.direction;
this.viewTransition = viewTransition;
@@ -145,6 +152,7 @@ export async function doPreparation(
navigationType: NavigationTypeString,
sourceElement: Element | undefined,
info: any,
+ signal: AbortSignal,
formData: FormData | undefined,
defaultLoader: (event: TransitionBeforePreparationEvent) => Promise
) {
@@ -156,6 +164,7 @@ export async function doPreparation(
sourceElement,
info,
window.document,
+ signal,
formData,
defaultLoader
);
@@ -172,7 +181,7 @@ export async function doPreparation(
return event;
}
-export async function doSwap(
+export function doSwap(
afterPreparation: BeforeEvent,
viewTransition: ViewTransition,
defaultSwap: (event: TransitionBeforeSwapEvent) => void
diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
index 8655d94c2b8e..4432c79344a2 100644
--- a/packages/astro/src/transitions/router.ts
+++ b/packages/astro/src/transitions/router.ts
@@ -8,6 +8,15 @@ type State = {
scrollY: number;
};
type Events = 'astro:page-load' | 'astro:after-swap';
+type Navigation = { controller: AbortController };
+type Transition = {
+ // The view transitions object (API and simulation)
+ viewTransition?: ViewTransition;
+ // Simulation: Whether transition was skipped
+ transitionSkipped: boolean;
+ // Simulation: The resolve function of the finished promise
+ viewTransitionFinished?: () => void;
+};
// Create bound versions of pushState/replaceState so that Partytown doesn't hijack them,
// which breaks Firefox.
@@ -33,15 +42,13 @@ export const transitionEnabledOnThisPage = () =>
const samePage = (thisLocation: URL, otherLocation: URL) =>
thisLocation.pathname === otherLocation.pathname && thisLocation.search === otherLocation.search;
+// The previous navigation that might still be in processing
+let mostRecentNavigation: Navigation | undefined;
+// The previous transition that might still be in processing
+let mostRecentTransition: Transition | undefined;
// When we traverse the history, the window.location is already set to the new location.
// This variable tells us where we came from
let originalLocation: URL;
-// The result of startViewTransition (browser or simulation)
-let viewTransition: ViewTransition | undefined;
-// skip transition flag for fallback simulation
-let skipTransition = false;
-// The resolve function of the finished promise for fallback simulation
-let viewTransitionFinished: () => void;
const triggerEvent = (name: Events) => document.dispatchEvent(new Event(name));
const onPageLoad = () => triggerEvent('astro:page-load');
@@ -83,7 +90,7 @@ if (inBrowser) {
currentHistoryIndex = history.state.index;
scrollTo({ left: history.state.scrollX, top: history.state.scrollY });
} else if (transitionEnabledOnThisPage()) {
- // This page is loaded from the browser addressbar or via a link from extern,
+ // This page is loaded from the browser address bar or via a link from extern,
// it needs a state in the history
replaceState({ index: currentHistoryIndex, scrollX, scrollY }, '');
history.scrollRestoration = 'manual';
@@ -148,7 +155,7 @@ function runScripts() {
return wait;
}
-// Add a new entry to the browser history. This also sets the new page in the browser addressbar.
+// Add a new entry to the browser history. This also sets the new page in the browser address bar.
// Sets the scroll position according to the hash fragment of the new location.
const moveToLocation = (
to: URL,
@@ -185,7 +192,7 @@ const moveToLocation = (
}
}
document.title = targetPageTitle;
- // now we are on the new page for non-history navigations!
+ // now we are on the new page for non-history navigation!
// (with history navigation page change happens before popstate is fired)
originalLocation = to;
@@ -253,6 +260,7 @@ function preloadStyleLinks(newDocument: Document) {
async function updateDOM(
preparationEvent: TransitionBeforePreparationEvent,
options: Options,
+ currentTransition: Transition,
historyState?: State,
fallback?: Fallback
) {
@@ -403,30 +411,45 @@ async function updateDOM(
const newAnimations = nextAnimations.filter(
(a) => !currentAnimations.includes(a) && !isInfinite(a)
);
- return Promise.all(newAnimations.map((a) => a.finished));
+ // Wait for all new animations to finish (resolved or rejected).
+ // Do not reject on canceled ones.
+ return Promise.allSettled(newAnimations.map((a) => a.finished));
}
- if (!skipTransition) {
- document.documentElement.setAttribute(DIRECTION_ATTR, preparationEvent.direction);
-
- if (fallback === 'animate') {
+ if (
+ fallback === 'animate' &&
+ !currentTransition.transitionSkipped &&
+ !preparationEvent.signal.aborted
+ ) {
+ try {
await animate('old');
+ } catch (err) {
+ // animate might reject as a consequence of a call to skipTransition()
+ // ignored on purpose
}
- } else {
- // that's what Chrome does
- throw new DOMException('Transition was skipped');
}
const pageTitleForBrowserHistory = document.title; // document.title will be overridden by swap()
- const swapEvent = await doSwap(preparationEvent, viewTransition!, defaultSwap);
+ const swapEvent = doSwap(preparationEvent, currentTransition.viewTransition!, defaultSwap);
moveToLocation(swapEvent.to, swapEvent.from, options, pageTitleForBrowserHistory, historyState);
triggerEvent(TRANSITION_AFTER_SWAP);
- if (fallback === 'animate' && !skipTransition) {
- animate('new').then(() => viewTransitionFinished());
+ if (fallback === 'animate') {
+ if (!currentTransition.transitionSkipped && !swapEvent.signal.aborted) {
+ animate('new').finally(() => currentTransition.viewTransitionFinished!());
+ } else {
+ currentTransition.viewTransitionFinished!();
+ }
}
}
+function abortAndRecreateMostRecentNavigation(): Navigation {
+ mostRecentNavigation?.controller.abort();
+ return (mostRecentNavigation = {
+ controller: new AbortController(),
+ });
+}
+
async function transition(
direction: Direction,
from: URL,
@@ -434,8 +457,17 @@ async function transition(
options: Options,
historyState?: State
) {
+ // The most recent navigation always has precendence
+ // Yes, there can be several navigation instances as the user can click links
+ // while we fetch content or simulate view transitions. Even synchronous creations are possible
+ // e.g. by calling navigate() from an transition event.
+ // Invariant: all but the most recent navigation are already aborted.
+
+ const currentNavigation = abortAndRecreateMostRecentNavigation();
+
// not ours
if (!transitionEnabledOnThisPage() || location.origin !== to.origin) {
+ if (currentNavigation === mostRecentNavigation) mostRecentNavigation = undefined;
location.href = to.href;
return;
}
@@ -452,6 +484,7 @@ async function transition(
if (samePage(from, to)) {
if ((direction !== 'back' && to.hash) || (direction === 'back' && from.hash)) {
moveToLocation(to, from, options, document.title, historyState);
+ if (currentNavigation === mostRecentNavigation) mostRecentNavigation = undefined;
return;
}
}
@@ -463,17 +496,23 @@ async function transition(
navigationType,
options.sourceElement,
options.info,
+ currentNavigation!.controller.signal,
options.formData,
defaultLoader
);
- if (prepEvent.defaultPrevented) {
- location.href = to.href;
+ if (prepEvent.defaultPrevented || prepEvent.signal.aborted) {
+ if (currentNavigation === mostRecentNavigation) mostRecentNavigation = undefined;
+ if (!prepEvent.signal.aborted) {
+ // not aborted -> delegate to browser
+ location.href = to.href;
+ }
+ // and / or exit
return;
}
async function defaultLoader(preparationEvent: TransitionBeforePreparationEvent) {
const href = preparationEvent.to.href;
- const init: RequestInit = {};
+ const init: RequestInit = { signal: preparationEvent.signal };
if (preparationEvent.formData) {
init.method = 'POST';
const form =
@@ -527,22 +566,57 @@ async function transition(
}
const links = preloadStyleLinks(preparationEvent.newDocument);
- links.length && (await Promise.all(links));
+ links.length && !preparationEvent.signal.aborted && (await Promise.all(links));
- if (import.meta.env.DEV)
- await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to);
+ if (import.meta.env.DEV && !preparationEvent.signal.aborted)
+ await prepareForClientOnlyComponents(
+ preparationEvent.newDocument,
+ preparationEvent.to,
+ preparationEvent.signal
+ );
+ }
+ async function abortAndRecreateMostRecentTransition(): Promise {
+ if (mostRecentTransition) {
+ if (mostRecentTransition.viewTransition) {
+ try {
+ mostRecentTransition.viewTransition.skipTransition();
+ } catch {
+ // might throw AbortError DOMException. Ignored on purpose.
+ }
+ try {
+ // UpdateCallbackDone might already been settled, i.e. if the previous transition finished updating the DOM.
+ // Could not take long, we wait for it to avoid parallel updates
+ // (which are very unlikely as long as swap() is not async).
+ await mostRecentTransition.viewTransition.updateCallbackDone;
+ } catch {
+ // There was an error in the update callback of the transition which we cancel.
+ // Ignored on purpose
+ }
+ }
+ }
+ return (mostRecentTransition = { transitionSkipped: false });
+ }
+
+ const currentTransition = await abortAndRecreateMostRecentTransition();
+
+ if (prepEvent.signal.aborted) {
+ if (currentNavigation === mostRecentNavigation) mostRecentNavigation = undefined;
+ return;
}
- skipTransition = false;
+ document.documentElement.setAttribute(DIRECTION_ATTR, prepEvent.direction);
if (supportsViewTransitions) {
- viewTransition = document.startViewTransition(
- async () => await updateDOM(prepEvent, options, historyState)
+ // This automatically cancels any previous transition
+ // We also already took care that the earlier update callback got through
+ currentTransition.viewTransition = document.startViewTransition(
+ async () => await updateDOM(prepEvent, options, currentTransition, historyState)
);
} else {
+ // Simulation mode requires a bit more manual work
const updateDone = (async () => {
- // immediatelly paused to setup the ViewTransition object for Fallback mode
- await new Promise((r) => setTimeout(r));
- await updateDOM(prepEvent, options, historyState, getFallback());
+ // Immediately paused to setup the ViewTransition object for Fallback mode
+ await Promise.resolve(); // hop through the micro task queue
+ await updateDOM(prepEvent, options, currentTransition, historyState, getFallback());
})();
// When the updateDone promise is settled,
@@ -557,26 +631,46 @@ async function transition(
//
// "finished" resolves after all animations are done.
- viewTransition = {
+ currentTransition.viewTransition = {
updateCallbackDone: updateDone, // this is about correct
ready: updateDone, // good enough
- finished: new Promise((r) => (viewTransitionFinished = r)), // see end of updateDOM
+ // Finished promise could have been done better: finished rejects iff updateDone does.
+ // Our simulation always resolves, never rejects.
+ finished: new Promise((r) => (currentTransition.viewTransitionFinished = r)), // see end of updateDOM
skipTransition: () => {
- skipTransition = true;
+ currentTransition.transitionSkipped = true;
+ // This cancels all animations of the simulation
+ document.documentElement.removeAttribute(OLD_NEW_ATTR);
},
};
}
-
- viewTransition.ready.then(async () => {
+ // In earlier versions was then'ed on viewTransition.ready which would not execute
+ // if the visual part of the transition has errors or was skipped
+ currentTransition.viewTransition.updateCallbackDone.finally(async () => {
await runScripts();
onPageLoad();
announce();
});
- viewTransition.finished.then(() => {
+ // finished.ready and finished.finally are the same for the simulation but not
+ // necessarily for native view transition, where finished rejects when updateCallbackDone does.
+ currentTransition.viewTransition.finished.finally(() => {
+ currentTransition.viewTransition = undefined;
+ if (currentTransition === mostRecentTransition) mostRecentTransition = undefined;
+ if (currentNavigation === mostRecentNavigation) mostRecentNavigation = undefined;
document.documentElement.removeAttribute(DIRECTION_ATTR);
document.documentElement.removeAttribute(OLD_NEW_ATTR);
});
- await viewTransition.ready;
+ try {
+ // Compatibility:
+ // In an earlier version we awaited viewTransition.ready, which includes animation setup.
+ // Scripts that depend on the view transition pseudo elements should hook on viewTransition.ready.
+ await currentTransition.viewTransition.updateCallbackDone;
+ } catch (e) {
+ // This log doesn't make it worse than before, where we got error messages about uncaught exceptions, which can't be catched when the trigger was a click or history traversal.
+ // Needs more investigation on root causes if errors still occur sporadically
+ const err = e as Error;
+ console.log('[astro]', err.name, err.message, err.stack);
+ }
}
let navigateOnServerWarned = false;
@@ -682,7 +776,11 @@ if (inBrowser) {
// Keep all styles that are potentially created by client:only components
// and required on the next page
-async function prepareForClientOnlyComponents(newDocument: Document, toLocation: URL) {
+async function prepareForClientOnlyComponents(
+ newDocument: Document,
+ toLocation: URL,
+ signal: AbortSignal
+) {
// Any client:only component on the next page?
if (newDocument.body.querySelector(`astro-island[client='only']`)) {
// Load the next page with an empty module loader cache
@@ -716,12 +814,14 @@ async function prepareForClientOnlyComponents(newDocument: Document, toLocation:
// return a promise that resolves when all astro-islands are hydrated
async function hydrationDone(loadingPage: HTMLIFrameElement) {
- await new Promise((r) =>
- loadingPage.contentWindow?.addEventListener('load', r, { once: true })
- );
-
+ if (!signal.aborted) {
+ await new Promise((r) =>
+ loadingPage.contentWindow?.addEventListener('load', r, { once: true })
+ );
+ }
return new Promise(async (r) => {
for (let count = 0; count <= 20; ++count) {
+ if (signal.aborted) break;
if (!loadingPage.contentDocument!.body.querySelector('astro-island[ssr]')) break;
await new Promise((r2) => setTimeout(r2, 50));
}