Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: conflict between rewrite and actions middleware #11052

Merged
merged 6 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading