fix(transitions router): no-op on the server (#8771)

* fix(transitions router): no-op on the server * factor out onPopState * add e2e test case * Apply suggestions from code review Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * use supportsViewTransitions * add changeset * warn on navigate() use during ssr * switch supportsViewTransitions to import.meta.env * correct typo * bring back import.meta.env * !import.meta.env.SSR -> inBrowser * Apply suggestions from code review Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com> --------- Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com>
withastro · Oct 10, 2023 · bd5aa1c · bd5aa1c
1 parent 03e6979
commit bd5aa1c
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 6 deletions.
diff --git a/.changeset/forty-singers-ring.md b/.changeset/forty-singers-ring.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixed an issue where the transitions router did not work within framework components.
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/components/ClickToNavigate.jsx b/packages/astro/e2e/fixtures/view-transitions/src/components/ClickToNavigate.jsx
@@ -0,0 +1,5 @@
+import React from 'react';
+import { navigate } from "astro:transitions/client";
+export default function ClickToNavigate({ to, id }) {
+    return <button id={id} onClick={() => navigate(to)}>Navigate to `{to}`</button>;
+}
diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/client-load.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/client-load.astro
@@ -0,0 +1,12 @@
+---
+import ClickToNavigate from "../components/ClickToNavigate.jsx"
+import { ViewTransitions } from "astro:transitions";
+---
+<html>
+	<head>
+		<ViewTransitions />
+	</head>
+	<body>
+		<ClickToNavigate id="react-client-load-navigate-button" to="/two" client:load/>
+	</body>
+</html>
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js
@@ -753,6 +753,21 @@ test.describe('View Transitions', () => {
 		await expect(p, 'should have content').toHaveText('Page 1');
 	});
 
+	test('Use the client side router in framework components', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/client-load'));
+
+		// the button is set to naviagte() to /two
+		const button = page.locator('#react-client-load-navigate-button');
+
+		await expect(button, 'should have content').toHaveText('Navigate to `/two`');
+
+		await button.click();
+
+		const p = page.locator('#two');
+
+		await expect(p, 'should have content').toHaveText('Page 2');
+	});
+
 	test('body inline scripts do not re-execute on navigation', async ({ page, astro }) => {
 		const errors = [];
 		page.addListener('pageerror', (err) => {

diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts
@@ -13,9 +13,14 @@ type Events = 'astro:page-load' | 'astro:after-swap';
 // only update history entries that are managed by us
 // leave other entries alone and do not accidently add state.
 const persistState = (state: State) => history.state && history.replaceState(state, '');
-export const supportsViewTransitions = !!document.startViewTransition;
+
+const inBrowser = import.meta.env.SSR === false;
+
+export const supportsViewTransitions = inBrowser && !!document.startViewTransition;
+
 export const transitionEnabledOnThisPage = () =>
-	!!document.querySelector('[name="astro-view-transitions-enabled"]');
+    inBrowser && !!document.querySelector('[name="astro-view-transitions-enabled"]');
+
 const samePage = (otherLocation: URL) =>
 	location.pathname === otherLocation.pathname && location.search === otherLocation.search;
 const triggerEvent = (name: Events) => document.dispatchEvent(new Event(name));
@@ -40,13 +45,17 @@ const announce = () => {
 		60
 	);
 };
+
 const PERSIST_ATTR = 'data-astro-transition-persist';
-const parser = new DOMParser();
+
+let parser: DOMParser
 
 // The History API does not tell you if navigation is forward or back, so
 // you can figure it using an index. On pushState the index is incremented so you
 // can use that to determine popstate if going forward or back.
 let currentHistoryIndex = 0;
+
+if (inBrowser) {
 if (history.state) {
 	// we reloaded a page with history state
 	// (e.g. history navigation from non-transition page or browser reload)
@@ -55,6 +64,8 @@ if (history.state) {
 } else if (transitionEnabledOnThisPage()) {
 	history.replaceState({ index: currentHistoryIndex, scrollX, scrollY, intraPage: false }, '');
 }
+}
+
 const throttle = (cb: (...args: any[]) => any, delay: number) => {
 	let wait = false;
 	// During the waiting time additional events are lost.
@@ -336,6 +347,8 @@ async function transition(
 		toLocation = new URL(response.redirected);
 	}
 
+	parser ??= new DOMParser()
+
 	const newDocument = parser.parseFromString(response.html, response.mediaType);
 	// The next line might look like a hack,
 	// but it is actually necessary as noscript elements
@@ -372,7 +385,21 @@ async function transition(
 	}
 }
 
+let navigateOnServerWarned = false;
+
 export function navigate(href: string, options?: Options) {
+
+	if (inBrowser === false) {
+		if (!navigateOnServerWarned) {
+			// instantiate an error for the stacktrace to show to user.
+			const warning = new Error("The view transtions client API was called during a server side render. This may be unintentional as the navigate() function is expected to be called in response to user interactions. Please make sure that your usage is correct.");
+			warning.name = "Warning";
+			console.warn(warning);
+			navigateOnServerWarned = true;
+		}
+		return;
+	}
+
 	// not ours
 	if (!transitionEnabledOnThisPage()) {
 		location.href = href;
@@ -390,8 +417,7 @@ export function navigate(href: string, options?: Options) {
 	}
 }
 
-if (supportsViewTransitions || getFallback() !== 'none') {
-	addEventListener('popstate', (ev) => {
+function onPopState(ev: PopStateEvent) {
 		if (!transitionEnabledOnThisPage() && ev.state) {
 			// The current page doesn't have View Transitions enabled
 			// but the page we navigate to does (because it set the state).
@@ -431,8 +457,11 @@ if (supportsViewTransitions || getFallback() !== 'none') {
 			currentHistoryIndex = nextIndex;
 			transition(direction, new URL(location.href), {}, state);
 		}
-	});
+	}
 
+if (inBrowser) {
+if (supportsViewTransitions || getFallback() !== 'none') {
+	addEventListener('popstate', onPopState);
 	addEventListener('load', onPageLoad);
 	// There's not a good way to record scroll position before a back button.
 	// So the way we do it is by listening to scrollend if supported, and if not continuously record the scroll position.
@@ -445,3 +474,4 @@ if (supportsViewTransitions || getFallback() !== 'none') {
 
 	markScriptsExec();
 }
+}