From 6da6d1db5e9d5c84c4d03f97a3fe2d1b3b13a92b Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sat, 27 Apr 2024 14:09:20 +0200 Subject: [PATCH 1/8] Detection and cancelation of previous navigations and view transitions --- .changeset/curly-spoons-pull.md | 5 + .../view-transitions/src/pages/abort.astro | 25 +++ packages/astro/e2e/view-transitions.test.js | 44 +++-- packages/astro/src/transitions/events.ts | 17 +- packages/astro/src/transitions/router.ts | 172 +++++++++++++----- 5 files changed, 200 insertions(+), 63 deletions(-) create mode 100644 .changeset/curly-spoons-pull.md create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro diff --git a/.changeset/curly-spoons-pull.md b/.changeset/curly-spoons-pull.md new file mode 100644 index 000000000000..8dafc0f5767c --- /dev/null +++ b/.changeset/curly-spoons-pull.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Detects overlapping navigations 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..c758a0e80aab --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro @@ -0,0 +1,25 @@ +--- +import { ViewTransitions } from 'astro:transitions'; +--- + + + + + +

Abort

+ + + diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 0913380c6377..a50ac02341eb 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1,3 +1,4 @@ +import exp from 'constants'; import { expect } from '@playwright/test'; import { testFactory, waitForHydrate } from './test-utils.js'; @@ -1426,21 +1427,32 @@ 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 wait /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'); + }); }); 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..3e543ca7ec5b 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -8,6 +8,17 @@ 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; + // The resolve function of the finished promise for fallback simulation + viewTransitionFinished?: () => void; + // for the simulation, the animations that are awaited + awaitedAnimations: Animation[]; +}; // Create bound versions of pushState/replaceState so that Partytown doesn't hijack them, // which breaks Firefox. @@ -33,15 +44,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'); @@ -253,6 +262,7 @@ function preloadStyleLinks(newDocument: Document) { async function updateDOM( preparationEvent: TransitionBeforePreparationEvent, options: Options, + currentTransition: Transition, historyState?: State, fallback?: Fallback ) { @@ -400,33 +410,46 @@ async function updateDOM( // Trigger view transition animations waiting for data-astro-transition-fallback document.documentElement.setAttribute(OLD_NEW_ATTR, phase); const nextAnimations = document.getAnimations(); - const newAnimations = nextAnimations.filter( + currentTransition.awaitedAnimations = nextAnimations.filter( (a) => !currentAnimations.includes(a) && !isInfinite(a) ); - return Promise.all(newAnimations.map((a) => a.finished)); + return Promise.all(currentTransition.awaitedAnimations.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) { + 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,49 @@ 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 && !preparationEvent.signal.aborted) + await prepareForClientOnlyComponents( + preparationEvent.newDocument, + preparationEvent.to, + preparationEvent.signal + ); + } + async function abortAndRecreateMostRecentTransition(): Promise { + if (mostRecentTransition) { + if (mostRecentTransition.viewTransition) { + mostRecentTransition.viewTransition.skipTransition(); + try { + // this might already been settled. If no the previous transition still updates the DOM. + // Could not take long, we wait for it. + await mostRecentTransition.viewTransition.updateCallbackDone; + } catch (err) { + // There was an error in the update callback of the transition we cancel. + // Ignored on purpose + } + } + } + return (mostRecentTransition = { transitionSkipped: false, awaitedAnimations: [] }); + } + + const currentTransition = await abortAndRecreateMostRecentTransition(); - if (import.meta.env.DEV) - await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to); + 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) + currentTransition.viewTransition = document.startViewTransition( + async () => await updateDOM(prepEvent, options, currentTransition, historyState) ); } else { 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(); + await updateDOM(prepEvent, options, currentTransition, historyState, getFallback()); })(); // When the updateDone promise is settled, @@ -557,26 +623,40 @@ 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 + // Could do better: finished rejects iff updateDone does. + // The simulation always resolves, never rejects. + finished: new Promise((r) => (currentTransition.viewTransitionFinished = r)), // see end of updateDOM skipTransition: () => { - skipTransition = true; + currentTransition.transitionSkipped = true; + currentTransition.awaitedAnimations.forEach((a) => a.cancel()); }, }; } - viewTransition.ready.then(async () => { + currentTransition.viewTransition.updateCallbackDone.then(async () => { await runScripts(); onPageLoad(); announce(); }); - viewTransition.finished.then(() => { + currentTransition.viewTransition.finished.finally(() => { + currentTransition.awaitedAnimations.length = 0; + currentTransition.viewTransition = undefined; + if (currentTransition === mostRecentTransition) mostRecentTransition = undefined; document.documentElement.removeAttribute(DIRECTION_ATTR); document.documentElement.removeAttribute(OLD_NEW_ATTR); }); - await viewTransition.ready; + try { + // In an earlier version wie awaited viewTransition.ready, which includes animation setup. + // Now we will only wait for swap, moveto and after-swap events + await currentTransition.viewTransition.updateCallbackDone; + } catch (err) { + // Prevent error message for uncaught DOMExceptions. + // But needs more investigation on root causes. + console.log("[astro]", err) + } } let navigateOnServerWarned = false; @@ -682,7 +762,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 @@ -697,7 +781,7 @@ async function prepareForClientOnlyComponents(newDocument: Document, toLocation: acc[key] = () => {}; return acc; }, {}); - await hydrationDone(nextPage); + await hydrationDone(nextPage, signal); const nextHead = nextPage.contentDocument?.head; if (nextHead) { @@ -715,13 +799,15 @@ 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 }) - ); - + async function hydrationDone(loadingPage: HTMLIFrameElement, signal: AbortSignal) { + 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)); } From 1eca32f81134252a9eb20f9255d5304c901e465c Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sat, 27 Apr 2024 17:12:06 +0200 Subject: [PATCH 2/8] typos and wording --- .changeset/curly-spoons-pull.md | 2 +- packages/astro/src/transitions/router.ts | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.changeset/curly-spoons-pull.md b/.changeset/curly-spoons-pull.md index 8dafc0f5767c..7d0d085fe475 100644 --- a/.changeset/curly-spoons-pull.md +++ b/.changeset/curly-spoons-pull.md @@ -2,4 +2,4 @@ "astro": patch --- -Detects overlapping navigations and view transitions and automatically aborts all but the most recent one. +Detects overlapping navigation and view transitions and automatically aborts all but the most recent one. diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 3e543ca7ec5b..03d0f9a41bd9 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -580,11 +580,11 @@ async function transition( if (mostRecentTransition.viewTransition) { mostRecentTransition.viewTransition.skipTransition(); try { - // this might already been settled. If no the previous transition still updates the DOM. + // This might already been settled, i.e. if the previous transition finished updating the DOM. // Could not take long, we wait for it. await mostRecentTransition.viewTransition.updateCallbackDone; } catch (err) { - // There was an error in the update callback of the transition we cancel. + // There was an error in the update callback of the transition which we cancel. // Ignored on purpose } } @@ -601,13 +601,16 @@ async function transition( document.documentElement.setAttribute(DIRECTION_ATTR, prepEvent.direction); if (supportsViewTransitions) { + // This automatically cancels any previous transition + // We also already take 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 () => { - // immediately paused to setup the ViewTransition object for Fallback mode - await Promise.resolve(); + // 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()); })(); @@ -626,8 +629,8 @@ async function transition( currentTransition.viewTransition = { updateCallbackDone: updateDone, // this is about correct ready: updateDone, // good enough - // Could do better: finished rejects iff updateDone does. - // The simulation always resolves, never rejects. + // 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: () => { currentTransition.transitionSkipped = true; @@ -635,8 +638,7 @@ async function transition( }, }; } - - currentTransition.viewTransition.updateCallbackDone.then(async () => { + currentTransition.viewTransition.ready.then(async () => { await runScripts(); onPageLoad(); announce(); From b038f9139cddf8a2964364da4d7f898ba01beab3 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sat, 27 Apr 2024 17:12:50 +0200 Subject: [PATCH 3/8] typos and wording --- packages/astro/src/transitions/router.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 03d0f9a41bd9..11eefb18ed4c 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -92,7 +92,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'; @@ -157,7 +157,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, @@ -194,7 +194,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; From 47e511fb50f1c76e1387fa4700f4a26e16db0e83 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sat, 27 Apr 2024 17:58:45 +0200 Subject: [PATCH 4/8] add test for animation cancelation --- .../view-transitions/src/pages/abort.astro | 4 ++++ .../view-transitions/src/pages/abort2.astro | 23 +++++++++++++++++++ packages/astro/e2e/view-transitions.test.js | 21 ++++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro index c758a0e80aab..7bee46f51a6d 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort.astro @@ -11,15 +11,19 @@ import { ViewTransitions } from 'astro:transitions'; 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..46836dc79f29 --- /dev/null +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro @@ -0,0 +1,23 @@ +--- +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 a50ac02341eb..3313819397e8 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1,6 +1,7 @@ import exp from 'constants'; import { expect } from '@playwright/test'; import { testFactory, waitForHydrate } from './test-utils.js'; +import { maxSatisfying } from 'semver'; const test = testFactory({ root: './fixtures/view-transitions/' }); @@ -1450,9 +1451,27 @@ test.describe('View Transitions', () => { // implemented in /abort: // clicks on slow loading page two // after short delay clicks on fast loading page one - // even after some wait /two should not show up + // 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 txt = ''; + page.on('console', (msg) => { + if (msg.text().startsWith('canceled')) { + txt += msg.text()+'\n'; + } + }); + 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'); + expect(txt).toBe('canceled astroFadeOut\ncanceled astroFadeIn\n'); + }); }); From 09b9cd2b624ad9c7ba3de74efef9bc043b5cacc1 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sun, 28 Apr 2024 19:36:39 +0200 Subject: [PATCH 5/8] second round --- .../view-transitions/src/pages/abort2.astro | 6 ++-- packages/astro/e2e/view-transitions.test.js | 15 ++++---- packages/astro/src/transitions/router.ts | 36 ++++++++++--------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro index 46836dc79f29..f4f118db8e14 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/abort2.astro @@ -15,9 +15,11 @@ import { ViewTransitions, fade } from 'astro:transitions'; import {navigate } from 'astro:transitions/client'; setTimeout(()=>{ - [...document.getAnimations()].forEach((a) => a.addEventListener('cancel', (e) => console.log('canceled', a.animationName))); + [...document.getAnimations()].forEach((a) => a.addEventListener('cancel', + (e) => console.log("[test]",e.type, a.animationName))); + console.log("[test] navigate to /one") navigate("/one"); }, 1000); - + console.log('[test] navigate to "."') navigate("/abort2"); diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 3313819397e8..e4a8bfadd9f4 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1,7 +1,7 @@ import exp from 'constants'; import { expect } from '@playwright/test'; -import { testFactory, waitForHydrate } from './test-utils.js'; import { maxSatisfying } from 'semver'; +import { testFactory, waitForHydrate } from './test-utils.js'; const test = testFactory({ root: './fixtures/view-transitions/' }); @@ -1458,11 +1458,9 @@ test.describe('View Transitions', () => { }); test('animation get canceled when view transition is interrupted', async ({ page, astro }) => { - let txt = ''; + let lines = []; page.on('console', (msg) => { - if (msg.text().startsWith('canceled')) { - txt += msg.text()+'\n'; - } + msg.text().startsWith("[test]") && lines.push(msg.text()); }); await page.goto(astro.resolveUrl('/abort2')); // implemented in /abort2: @@ -1472,6 +1470,11 @@ test.describe('View Transitions', () => { 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'); - expect(txt).toBe('canceled astroFadeOut\ncanceled astroFadeIn\n'); + + // 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/router.ts b/packages/astro/src/transitions/router.ts index 11eefb18ed4c..c22527e7fb9f 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -16,8 +16,6 @@ type Transition = { transitionSkipped: boolean; // The resolve function of the finished promise for fallback simulation viewTransitionFinished?: () => void; - // for the simulation, the animations that are awaited - awaitedAnimations: Animation[]; }; // Create bound versions of pushState/replaceState so that Partytown doesn't hijack them, @@ -410,10 +408,10 @@ async function updateDOM( // Trigger view transition animations waiting for data-astro-transition-fallback document.documentElement.setAttribute(OLD_NEW_ATTR, phase); const nextAnimations = document.getAnimations(); - currentTransition.awaitedAnimations = nextAnimations.filter( + const newAnimations = nextAnimations.filter( (a) => !currentAnimations.includes(a) && !isInfinite(a) ); - return Promise.all(currentTransition.awaitedAnimations.map((a) => a.finished)); + return Promise.allSettled(newAnimations.map((a) => a.finished)); } if ( @@ -435,7 +433,7 @@ async function updateDOM( triggerEvent(TRANSITION_AFTER_SWAP); if (fallback === 'animate') { - if (!currentTransition.transitionSkipped) { + if (!currentTransition.transitionSkipped && !swapEvent.signal.aborted) { animate('new').finally(() => currentTransition.viewTransitionFinished!()); } else { currentTransition.viewTransitionFinished!(); @@ -581,7 +579,8 @@ async function transition( mostRecentTransition.viewTransition.skipTransition(); try { // This might already been settled, i.e. if the previous transition finished updating the DOM. - // Could not take long, we wait for it. + // 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 (err) { // There was an error in the update callback of the transition which we cancel. @@ -589,7 +588,7 @@ async function transition( } } } - return (mostRecentTransition = { transitionSkipped: false, awaitedAnimations: [] }); + return (mostRecentTransition = { transitionSkipped: false }); } const currentTransition = await abortAndRecreateMostRecentTransition(); @@ -634,30 +633,35 @@ async function transition( finished: new Promise((r) => (currentTransition.viewTransitionFinished = r)), // see end of updateDOM skipTransition: () => { currentTransition.transitionSkipped = true; - currentTransition.awaitedAnimations.forEach((a) => a.cancel()); + document.documentElement.removeAttribute(OLD_NEW_ATTR); }, }; } - currentTransition.viewTransition.ready.then(async () => { + // in earlier versions was then'ed on viewTransition.ready which would not execute + // if the visual prt of the transition has errors or was skipped + currentTransition.viewTransition.updateCallbackDone.finally(async () => { await runScripts(); onPageLoad(); announce(); }); + // 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.awaitedAnimations.length = 0; currentTransition.viewTransition = undefined; if (currentTransition === mostRecentTransition) mostRecentTransition = undefined; document.documentElement.removeAttribute(DIRECTION_ATTR); document.documentElement.removeAttribute(OLD_NEW_ATTR); }); try { - // In an earlier version wie awaited viewTransition.ready, which includes animation setup. - // Now we will only wait for swap, moveto and after-swap events + // 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 (err) { - // Prevent error message for uncaught DOMExceptions. - // But needs more investigation on root causes. - console.log("[astro]", err) + } catch (e) { + // This log doesn't make it worse than before, where we got error messages about uncaught exception + // Needs more investigation on root causes. + const err = e as Error; + console.log('[astro]', err.name, err.message, err.stack); } } From d5e67b3ab4a58a417096dd4a242b0da0be85c42a Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sun, 28 Apr 2024 20:33:37 +0200 Subject: [PATCH 6/8] final touches --- packages/astro/e2e/view-transitions.test.js | 8 +++---- packages/astro/src/transitions/router.ts | 25 +++++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index e4a8bfadd9f4..65f136531574 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -1,6 +1,4 @@ -import exp from 'constants'; import { expect } from '@playwright/test'; -import { maxSatisfying } from 'semver'; import { testFactory, waitForHydrate } from './test-utils.js'; const test = testFactory({ root: './fixtures/view-transitions/' }); @@ -1460,7 +1458,7 @@ test.describe('View Transitions', () => { 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()); + msg.text().startsWith('[test]') && lines.push(msg.text()); }); await page.goto(astro.resolveUrl('/abort2')); // implemented in /abort2: @@ -1475,6 +1473,8 @@ test.describe('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'); + 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/router.ts b/packages/astro/src/transitions/router.ts index c22527e7fb9f..1582684c2e4b 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -14,7 +14,7 @@ type Transition = { viewTransition?: ViewTransition; // Simulation: Whether transition was skipped transitionSkipped: boolean; - // The resolve function of the finished promise for fallback simulation + // Simulation: The resolve function of the finished promise viewTransitionFinished?: () => void; }; @@ -411,6 +411,8 @@ async function updateDOM( const newAnimations = nextAnimations.filter( (a) => !currentAnimations.includes(a) && !isInfinite(a) ); + // Wait for all new animations to finish (resolved or rejected). + // Do not reject on canceled ones. return Promise.allSettled(newAnimations.map((a) => a.finished)); } @@ -576,13 +578,17 @@ async function transition( async function abortAndRecreateMostRecentTransition(): Promise { if (mostRecentTransition) { if (mostRecentTransition.viewTransition) { - mostRecentTransition.viewTransition.skipTransition(); try { - // This might already been settled, i.e. if the previous transition finished updating the DOM. + 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 (err) { + } catch { // There was an error in the update callback of the transition which we cancel. // Ignored on purpose } @@ -601,7 +607,7 @@ async function transition( document.documentElement.setAttribute(DIRECTION_ATTR, prepEvent.direction); if (supportsViewTransitions) { // This automatically cancels any previous transition - // We also already take care that the earlier update callback got through + // We also already took care that the earlier update callback got through currentTransition.viewTransition = document.startViewTransition( async () => await updateDOM(prepEvent, options, currentTransition, historyState) ); @@ -633,12 +639,13 @@ async function transition( finished: new Promise((r) => (currentTransition.viewTransitionFinished = r)), // see end of updateDOM skipTransition: () => { currentTransition.transitionSkipped = true; + // This cancels all animations of the simulation document.documentElement.removeAttribute(OLD_NEW_ATTR); }, }; } - // in earlier versions was then'ed on viewTransition.ready which would not execute - // if the visual prt of the transition has errors or was skipped + // 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(); @@ -658,8 +665,8 @@ async function transition( // 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 exception - // Needs more investigation on root causes. + // 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); } From 46fdbeaca899bd791b8d176e3472094e89d96dfe Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Sun, 28 Apr 2024 20:54:16 +0200 Subject: [PATCH 7/8] final final touches --- packages/astro/src/transitions/router.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 1582684c2e4b..017f825a3cbc 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -794,7 +794,7 @@ async function prepareForClientOnlyComponents( acc[key] = () => {}; return acc; }, {}); - await hydrationDone(nextPage, signal); + await hydrationDone(nextPage); const nextHead = nextPage.contentDocument?.head; if (nextHead) { @@ -812,7 +812,7 @@ async function prepareForClientOnlyComponents( } // return a promise that resolves when all astro-islands are hydrated - async function hydrationDone(loadingPage: HTMLIFrameElement, signal: AbortSignal) { + async function hydrationDone(loadingPage: HTMLIFrameElement) { if (!signal.aborted) { await new Promise((r) => loadingPage.contentWindow?.addEventListener('load', r, { once: true }) From cc3b8c927ea43fa680472bb3937813df36dbf925 Mon Sep 17 00:00:00 2001 From: Martin Trapp <94928215+martrapp@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:16:30 +0200 Subject: [PATCH 8/8] Clear the most recent navigation after view transition finished --- packages/astro/src/transitions/router.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index 017f825a3cbc..4432c79344a2 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -656,6 +656,7 @@ async function transition( 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); });