From a05ca38c2cf327ae9130ee1c139a0e510b9da50a Mon Sep 17 00:00:00 2001
From: Florian Lefebvre <contact@florian-lefebvre.dev>
Date: Wed, 15 May 2024 18:49:45 +0200
Subject: [PATCH] fix: conflict between rewrite and actions middleware (#11052)

* fix: conflict between rewrite and actions middleware

* Update middleware.ts

* fix: short circuit middleware if locals already defined

* chore(test): remove atkinson font refs

* feat(test): progressive fallbacks

* chore: remove unneeded conditional

---------

Co-authored-by: bholmesdev <hey@bholmes.dev>
---
 .changeset/khaki-papayas-knock.md             |  5 ++
 packages/astro/e2e/actions-blog.test.js       | 69 +++++++++++++++----
 .../src/components/BaseHead.astro             |  4 --
 .../src/components/PostComment.tsx            |  8 ++-
 .../src/pages/blog/[...slug].astro            | 20 +++++-
 .../astro/src/actions/runtime/middleware.ts   |  9 ++-
 6 files changed, 90 insertions(+), 25 deletions(-)
 create mode 100644 .changeset/khaki-papayas-knock.md

diff --git a/.changeset/khaki-papayas-knock.md b/.changeset/khaki-papayas-knock.md
new file mode 100644
index 0000000000..29ee5ce05e
--- /dev/null
+++ b/.changeset/khaki-papayas-knock.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes a case where rewriting would conflict with the actions internal middleware
diff --git a/packages/astro/e2e/actions-blog.test.js b/packages/astro/e2e/actions-blog.test.js
index 1fc7942796..fe4660b817 100644
--- a/packages/astro/e2e/actions-blog.test.js
+++ b/packages/astro/e2e/actions-blog.test.js
@@ -26,30 +26,68 @@ test.describe('Astro Actions - Blog', () => {
 	test('Comment action - validation error', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/blog/first-post/'));
 
-		const authorInput = page.locator('input[name="author"]');
-		const bodyInput = page.locator('textarea[name="body"]');
+		const form = page.getByTestId('client');
+		const authorInput = form.locator('input[name="author"]');
+		const bodyInput = form.locator('textarea[name="body"]');
 
 		await authorInput.fill('Ben');
 		await bodyInput.fill('Too short');
 
-		const submitButton = page.getByLabel('Post comment');
+		const submitButton = form.getByRole('button');
 		await submitButton.click();
 
-		await expect(page.locator('p[data-error="body"]')).toBeVisible();
+		await expect(form.locator('p[data-error="body"]')).toBeVisible();
+	});
+
+
+	test('Comment action - progressive fallback validation error', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/blog/first-post/'));
+
+		const form = page.getByTestId('progressive-fallback');
+		const authorInput = form.locator('input[name="author"]');
+		const bodyInput = form.locator('textarea[name="body"]');
+
+		await authorInput.fill('Ben');
+		await bodyInput.fill('Too short');
+
+		const submitButton = form.getByRole('button');
+		await submitButton.click();
+
+		await expect(form.locator('p[data-error="body"]')).toBeVisible();
+	});
+
+test('Comment action - progressive fallback success', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/blog/first-post/'));
+
+		const form = page.getByTestId('progressive-fallback');
+		const authorInput = form.locator('input[name="author"]');
+		const bodyInput = form.locator('textarea[name="body"]');
+
+		const body = 'Fallback - This should be long enough.';
+		await authorInput.fill('Ben');
+		await bodyInput.fill(body);
+
+		const submitButton = form.getByRole('button');
+		await submitButton.click();
+
+		const comments = page.getByTestId('server-comments');
+		await expect(comments).toBeVisible();
+		await expect(comments).toContainText(body);
 	});
 
 	test('Comment action - custom error', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/blog/first-post/?commentPostIdOverride=bogus'));
 
-		const authorInput = page.locator('input[name="author"]');
-		const bodyInput = page.locator('textarea[name="body"]');
+		const form = page.getByTestId('client');
+		const authorInput = form.locator('input[name="author"]');
+		const bodyInput = form.locator('textarea[name="body"]');
 		await authorInput.fill('Ben');
 		await bodyInput.fill('This should be long enough.');
 
-		const submitButton = page.getByLabel('Post comment');
+		const submitButton = form.getByRole('button');
 		await submitButton.click();
 
-		const unexpectedError = page.locator('p[data-error="unexpected"]');
+		const unexpectedError = form.locator('p[data-error="unexpected"]');
 		await expect(unexpectedError).toBeVisible();
 		await expect(unexpectedError).toContainText('NOT_FOUND: Post not found');
 	});
@@ -57,18 +95,19 @@ test.describe('Astro Actions - Blog', () => {
 	test('Comment action - success', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/blog/first-post/'));
 
-		const authorInput = page.locator('input[name="author"]');
-		const bodyInput = page.locator('textarea[name="body"]');
+		const form = page.getByTestId('client');
+		const authorInput = form.locator('input[name="author"]');
+		const bodyInput = form.locator('textarea[name="body"]');
 
-		const body = 'This should be long enough.';
+		const body = 'Client: This should be long enough.';
 		await authorInput.fill('Ben');
 		await bodyInput.fill(body);
 
-		const submitButton = page.getByLabel('Post comment');
+		const submitButton = form.getByRole('button');
 		await submitButton.click();
 
-		const comment = await page.getByTestId('comment');
-		await expect(comment).toBeVisible();
-		await expect(comment).toContainText(body);
+		const comments = page.getByTestId('client-comments');
+		await expect(comments).toBeVisible();
+		await expect(comments).toContainText(body);
 	});
 });
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/components/BaseHead.astro b/packages/astro/e2e/fixtures/actions-blog/src/components/BaseHead.astro
index 344124012b..5a2114003d 100644
--- a/packages/astro/e2e/fixtures/actions-blog/src/components/BaseHead.astro
+++ b/packages/astro/e2e/fixtures/actions-blog/src/components/BaseHead.astro
@@ -20,10 +20,6 @@ const { title, description, image = '/blog-placeholder-1.jpg' } = Astro.props;
 <link rel="icon" type="image/svg+xml" href="/favicon.svg" />
 <meta name="generator" content={Astro.generator} />
 
-<!-- Font preloads -->
-<link rel="preload" href="/fonts/atkinson-regular.woff" as="font" type="font/woff" crossorigin />
-<link rel="preload" href="/fonts/atkinson-bold.woff" as="font" type="font/woff" crossorigin />
-
 <!-- Canonical URL -->
 <link rel="canonical" href={canonicalURL} />
 
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx b/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx
index 87d2154893..f73d152e12 100644
--- a/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx
+++ b/packages/astro/e2e/fixtures/actions-blog/src/components/PostComment.tsx
@@ -16,6 +16,7 @@ export function PostComment({
 		<>
 			<form
 				method="POST"
+				data-testid="client"
 				onSubmit={async (e) => {
 					e.preventDefault();
 					const form = e.target as HTMLFormElement;
@@ -34,7 +35,7 @@ export function PostComment({
 				{unexpectedError && <p data-error="unexpected" style={{ color: 'red' }}>{unexpectedError}</p>}
 				<input {...getActionProps(actions.blog.comment)} />
 				<input type="hidden" name="postId" value={postId} />
-				<label className="sr-only" htmlFor="author">
+				<label htmlFor="author">
 					Author
 				</label>
 				<input id="author" type="text" name="author" placeholder="Your name" />
@@ -44,13 +45,13 @@ export function PostComment({
 						{bodyError}
 					</p>
 				)}
-				<button aria-label="Post comment" type="submit">
+				<button type="submit">
 					Post
 				</button>
 			</form>
+			<div data-testid="client-comments">
 			{comments.map((c) => (
 				<article
-					data-testid="comment"
 					key={c.body}
 					style={{
 						border: '2px solid color-mix(in srgb, var(--accent), transparent 80%)',
@@ -63,6 +64,7 @@ export function PostComment({
 					<p>{c.author}</p>
 				</article>
 			))}
+			</div>
 		</>
 	);
 }
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro b/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
index b490d980a2..d7eac0c4cd 100644
--- a/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
+++ b/packages/astro/e2e/fixtures/actions-blog/src/pages/blog/[...slug].astro
@@ -4,7 +4,7 @@ import BlogPost from '../../layouts/BlogPost.astro';
 import { db, eq, Comment, Likes } from 'astro:db';
 import { Like } from '../../components/Like';
 import { PostComment } from '../../components/PostComment';
-import { actions } from 'astro:actions';
+import { actions, getActionProps } from 'astro:actions';
 import { isInputError } from 'astro:actions';
 
 export const prerender = false;
@@ -45,7 +45,23 @@ const commentPostIdOverride = Astro.url.searchParams.get('commentPostIdOverride'
 			: undefined}
 		client:load
 	/>
-	<div>
+	<form method="POST" data-testid="progressive-fallback">
+		<input {...getActionProps(actions.blog.comment)} />
+		<input type="hidden" name="postId" value={post.id} />
+		<label for="fallback-author">
+			Author
+		</label>
+			<input id="fallback-author" type="text" name="author" required />
+		<label for="fallback-body" class="sr-only">
+			Comment
+		</label>
+		<textarea id="fallback-body" rows={10} name="body" required></textarea>
+		{isInputError(comment?.error) && comment.error.fields.body && (
+			<p class="error" data-error="body">{comment.error.fields.body.toString()}</p>
+		)}
+		<button type="submit">Post Comment</button>
+	</form>
+	<div data-testid="server-comments">
 		{
 			comments.map((c) => (
 				<article>
diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts
index 3241e0ac40..fced367aec 100644
--- a/packages/astro/src/actions/runtime/middleware.ts
+++ b/packages/astro/src/actions/runtime/middleware.ts
@@ -12,14 +12,21 @@ export type Locals = {
 
 export const onRequest = defineMiddleware(async (context, next) => {
 	const locals = context.locals as Locals;
+	// Actions middleware may have run already after a path rewrite.
+	// See https://github.com/withastro/roadmap/blob/feat/reroute/proposals/0047-rerouting.md#ctxrewrite
+	// `_actionsInternal` is the same for every page,
+	// so short circuit if already defined.
+	if (locals._actionsInternal) return next();
+
 	const { request, url } = context;
 	const contentType = request.headers.get('Content-Type');
 
 	// Avoid double-handling with middleware when calling actions directly.
 	if (url.pathname.startsWith('/_actions')) return nextWithLocalsStub(next, locals);
 
-	if (!contentType || !hasContentType(contentType, formContentTypes))
+	if (!contentType || !hasContentType(contentType, formContentTypes)) {
 		return nextWithLocalsStub(next, locals);
+	}
 
 	const formData = await request.clone().formData();
 	const actionPath = formData.get('_astroAction');