Skip to content

Commit

Permalink
fix: conflict between rewrite and actions middleware (#11052)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
florian-lefebvre and bholmesdev committed May 15, 2024
1 parent 5a48d53 commit a05ca38
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/khaki-papayas-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes a case where rewriting would conflict with the actions internal middleware
69 changes: 54 additions & 15 deletions packages/astro/e2e/actions-blog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,49 +26,88 @@ 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');
});

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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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} />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export function PostComment({
<>
<form
method="POST"
data-testid="client"
onSubmit={async (e) => {
e.preventDefault();
const form = e.target as HTMLFormElement;
Expand All @@ -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" />
Expand All @@ -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%)',
Expand All @@ -63,6 +64,7 @@ export function PostComment({
<p>{c.author}</p>
</article>
))}
</div>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>
Expand Down
9 changes: 8 additions & 1 deletion packages/astro/src/actions/runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit a05ca38

Please sign in to comment.