Increase consistency between navigation with and without ViewTransiti…

…ons (#9279) * Greater consistency between navigations with and without ViewTransitions * hmpf * formatting and test fixing * An attempt to explain * fixed tests * explain the event listener * Update .changeset/nervous-beans-peel.md Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com> --------- Co-authored-by: Matthew Phillips <matthew@skypack.dev> Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
withastro · Dec 4, 2023 · 6a9669b · 6a9669b
1 parent 8a228fc
commit 6a9669b
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 62 deletions.
diff --git a/.changeset/nervous-beans-peel.md b/.changeset/nervous-beans-peel.md
@@ -0,0 +1,6 @@
+---
+'astro': minor
+---
+
+Improves consistency between navigations with and without `<ViewTransitions>`. See [#9279](https://github.com/withastro/astro/pull/9279) for more details.
+
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/long-page.astro
@@ -51,3 +51,9 @@ import Layout from '../components/Layout.astro';
 		Libero id faucibus nisl tincidunt eget nullam non. Faucibus a pellentesque sit amet porttitor eget dolor. Posuere urna nec tincidunt praesent semper feugiat nibh sed. Suspendisse ultrices gravida dictum fusce. Porttitor eget dolor morbi non arcu. Neque egestas congue quisque egestas diam in. Suscipit tellus mauris a diam maecenas sed enim ut sem. Luctus accumsan tortor posuere ac. Tortor posuere ac ut consequat semper viverra. Egestas tellus rutrum tellus pellentesque eu tincidunt tortor aliquam nulla. Senectus et netus et malesuada fames ac turpis egestas. Sed libero enim sed faucibus turpis in eu mi bibendum. Sollicitudin tempor id eu nisl nunc mi.
 	</article>
 </Layout>
+<script is:inline>
+	// let playwright know when navigate() is done
+	document.addEventListener('astro:before-swap', (e) => {
+		e.viewTransition.ready.then(()=>console.log("ready"))
+	});
+</script>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -394,7 +394,10 @@ test.describe('View Transitions', () => {
 		await expect(locator).toBeInViewport();
 
 		// Scroll back to top
+		// back returns immediately, but we need to wait for navigate() to complete
+		const waitForReady = page.waitForEvent('console');
 		await page.goBack();
+		await waitForReady;
 		locator = page.locator('#longpage');
 		await expect(locator).toBeInViewport();
 

diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
@@ -25,9 +25,7 @@ export const transitionEnabledOnThisPage = () =>
 	inBrowser && !!document.querySelector('[name="astro-view-transitions-enabled"]');
 
 const samePage = (thisLocation: URL, otherLocation: URL) =>
-	thisLocation.origin === otherLocation.origin &&
-	thisLocation.pathname === otherLocation.pathname &&
-	thisLocation.search === otherLocation.search;
+	thisLocation.pathname === otherLocation.pathname && thisLocation.search === otherLocation.search;
 
 // When we traverse the history, the window.location is already set to the new location.
 // This variable tells us where we came from
@@ -418,16 +416,22 @@ async function transition(
 	options: Options,
 	historyState?: State
 ) {
+	// not ours
+	if (!transitionEnabledOnThisPage() || location.origin !== to.origin) {
+		location.href = to.href;
+		return;
+	}
+
 	const navigationType = historyState
 		? 'traverse'
 		: options.history === 'replace'
 		  ? 'replace'
 		  : 'push';
 
-	if (samePage(from, to) && !options.formData /* not yet: && to.hash*/) {
-		if (navigationType !== 'traverse') {
-			updateScrollPosition({ scrollX, scrollY });
-		}
+	if (navigationType !== 'traverse') {
+		updateScrollPosition({ scrollX, scrollY });
+	}
+	if (samePage(from, to) && !!to.hash) {
 		moveToLocation(to, from, options, historyState);
 		return;
 	}
@@ -447,61 +451,48 @@ async function transition(
 		return;
 	}
 
-	function pageMustReload(preparationEvent: TransitionBeforePreparationEvent) {
-		return (
-			preparationEvent.to.hash === '' ||
-			!samePage(preparationEvent.from, preparationEvent.to) ||
-			preparationEvent.sourceElement instanceof HTMLFormElement
-		);
-	}
-
 	async function defaultLoader(preparationEvent: TransitionBeforePreparationEvent) {
-		if (pageMustReload(preparationEvent)) {
-			const href = preparationEvent.to.href;
-			const init: RequestInit = {};
-			if (preparationEvent.formData) {
-				init.method = 'POST';
-				init.body = preparationEvent.formData;
-			}
-			const response = await fetchHTML(href, init);
-			// If there is a problem fetching the new page, just do an MPA navigation to it.
-			if (response === null) {
-				preparationEvent.preventDefault();
-				return;
-			}
-			// if there was a redirection, show the final URL in the browser's address bar
-			if (response.redirected) {
-				preparationEvent.to = new URL(response.redirected);
-			}
-
-			parser ??= new DOMParser();
-
-			preparationEvent.newDocument = parser.parseFromString(response.html, response.mediaType);
-			// The next line might look like a hack,
-			// but it is actually necessary as noscript elements
-			// and their contents are returned as markup by the parser,
-			// see https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString
-			preparationEvent.newDocument.querySelectorAll('noscript').forEach((el) => el.remove());
+		const href = preparationEvent.to.href;
+		const init: RequestInit = {};
+		if (preparationEvent.formData) {
+			init.method = 'POST';
+			init.body = preparationEvent.formData;
+		}
+		const response = await fetchHTML(href, init);
+		// If there is a problem fetching the new page, just do an MPA navigation to it.
+		if (response === null) {
+			preparationEvent.preventDefault();
+			return;
+		}
+		// if there was a redirection, show the final URL in the browser's address bar
+		if (response.redirected) {
+			preparationEvent.to = new URL(response.redirected);
+		}
 
-			// If ViewTransitions is not enabled on the incoming page, do a full page load to it.
-			// Unless this was a form submission, in which case we do not want to trigger another mutation.
-			if (
-				!preparationEvent.newDocument.querySelector('[name="astro-view-transitions-enabled"]') &&
-				!preparationEvent.formData
-			) {
-				preparationEvent.preventDefault();
-				return;
-			}
+		parser ??= new DOMParser();
 
-			const links = preloadStyleLinks(preparationEvent.newDocument);
-			links.length && (await Promise.all(links));
+		preparationEvent.newDocument = parser.parseFromString(response.html, response.mediaType);
+		// The next line might look like a hack,
+		// but it is actually necessary as noscript elements
+		// and their contents are returned as markup by the parser,
+		// see https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString
+		preparationEvent.newDocument.querySelectorAll('noscript').forEach((el) => el.remove());
 
-			if (import.meta.env.DEV)
-				await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to);
-		} else {
-			preparationEvent.newDocument = document;
+		// If ViewTransitions is not enabled on the incoming page, do a full page load to it.
+		// Unless this was a form submission, in which case we do not want to trigger another mutation.
+		if (
+			!preparationEvent.newDocument.querySelector('[name="astro-view-transitions-enabled"]') &&
+			!preparationEvent.formData
+		) {
+			preparationEvent.preventDefault();
 			return;
 		}
+
+		const links = preloadStyleLinks(preparationEvent.newDocument);
+		links.length && (await Promise.all(links));
+
+		if (import.meta.env.DEV)
+			await prepareForClientOnlyComponents(preparationEvent.newDocument, preparationEvent.to);
 	}
 
 	skipTransition = false;
@@ -566,12 +557,6 @@ export async function navigate(href: string, options?: Options) {
 		}
 		return;
 	}
-
-	// not ours
-	if (!transitionEnabledOnThisPage()) {
-		location.href = href;
-		return;
-	}
 	await transition('forward', originalLocation, new URL(href, location.href), options ?? {});
 }