From 6d3e04a9598a2de8ccc153911fd6ab629abcaf7a Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Mon, 18 Dec 2023 19:16:04 -0500 Subject: [PATCH 1/8] Respect forms with enctype set for view transitions --- packages/astro/components/ViewTransitions.astro | 13 ++++++++++++- packages/astro/src/transitions/events.ts | 10 +++++----- packages/astro/src/transitions/router.ts | 8 ++++---- packages/astro/src/transitions/types.ts | 2 +- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 310f1865a92c..00376a57826f 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -117,7 +117,18 @@ const { fallback = 'animate' } = Astro.props; url.search = params.toString(); action = url.toString(); } else { - options.formData = formData; + // Form elements without enctype explicitly set default to application/x-www-form-urlencoded. + // In order to maintain compatibility with Astro 4.x, we need to check the value of enctype + // on the attributes property rather than accessing .enctype directly. Astro 5.x may + // introduce defaulting to application/x-www-form-urlencoded as a breaking change, and then + // we can access .enctype directly. + // + // Note: getNamedItem can return null in real life, even if TypeScript doesn't think so. + const enctype = form.attributes.getNamedItem('enctype')?.value; + options.body = + enctype === 'application/x-www-form-urlencoded' + ? new URLSearchParams(formData as any) + : formData; } ev.preventDefault(); diff --git a/packages/astro/src/transitions/events.ts b/packages/astro/src/transitions/events.ts index b3921b31f0c9..691352d017f8 100644 --- a/packages/astro/src/transitions/events.ts +++ b/packages/astro/src/transitions/events.ts @@ -66,7 +66,7 @@ export const isTransitionBeforePreparationEvent = ( value: any ): value is TransitionBeforePreparationEvent => value.type === TRANSITION_BEFORE_PREPARATION; export class TransitionBeforePreparationEvent extends BeforeEvent { - formData: FormData | undefined; + body: FormData | URLSearchParams | undefined; loader: () => Promise; constructor( from: URL, @@ -76,7 +76,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent { sourceElement: Element | undefined, info: any, newDocument: Document, - formData: FormData | undefined, + body: FormData | URLSearchParams | undefined, loader: (event: TransitionBeforePreparationEvent) => Promise ) { super( @@ -90,7 +90,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent { info, newDocument ); - this.formData = formData; + this.body = body; this.loader = loader.bind(this, this); Object.defineProperties(this, { formData: { enumerable: true }, @@ -145,7 +145,7 @@ export async function doPreparation( navigationType: NavigationTypeString, sourceElement: Element | undefined, info: any, - formData: FormData | undefined, + body: FormData | URLSearchParams | undefined, defaultLoader: (event: TransitionBeforePreparationEvent) => Promise ) { const event = new TransitionBeforePreparationEvent( @@ -156,7 +156,7 @@ export async function doPreparation( sourceElement, info, window.document, - formData, + body, defaultLoader ); if (document.dispatchEvent(event)) { diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index e710c2e1b2e8..ca160fa5d4c6 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -449,7 +449,7 @@ async function transition( navigationType, options.sourceElement, options.info, - options.formData, + options.body, defaultLoader ); if (prepEvent.defaultPrevented) { @@ -460,9 +460,9 @@ async function transition( async function defaultLoader(preparationEvent: TransitionBeforePreparationEvent) { const href = preparationEvent.to.href; const init: RequestInit = {}; - if (preparationEvent.formData) { + if (preparationEvent.body) { init.method = 'POST'; - init.body = preparationEvent.formData; + init.body = preparationEvent.body; } const response = await fetchHTML(href, init); // If there is a problem fetching the new page, just do an MPA navigation to it. @@ -488,7 +488,7 @@ async function transition( // 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.body ) { preparationEvent.preventDefault(); return; diff --git a/packages/astro/src/transitions/types.ts b/packages/astro/src/transitions/types.ts index 0e70825e5967..9ad2ecdbbcf1 100644 --- a/packages/astro/src/transitions/types.ts +++ b/packages/astro/src/transitions/types.ts @@ -5,6 +5,6 @@ export type Options = { history?: 'auto' | 'push' | 'replace'; info?: any; state?: any; - formData?: FormData; + body?: FormData | URLSearchParams; sourceElement?: Element; // more than HTMLElement, e.g. SVGAElement }; From 1cc5fd2eb102adbd34d8118900307773801d714e Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Mon, 18 Dec 2023 19:21:37 -0500 Subject: [PATCH 2/8] Add changeset --- .changeset/rude-geckos-rush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rude-geckos-rush.md diff --git a/.changeset/rude-geckos-rush.md b/.changeset/rude-geckos-rush.md new file mode 100644 index 000000000000..18fbdb831bf1 --- /dev/null +++ b/.changeset/rude-geckos-rush.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Respect forms with enctype set for view transitions From a2e4832c51e86b5ec567de5edad6264d97c8ac46 Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 19 Dec 2023 10:31:08 -0500 Subject: [PATCH 3/8] Revert "Respect forms with enctype set for view transitions" This reverts commit 6d3e04a9598a2de8ccc153911fd6ab629abcaf7a. --- packages/astro/components/ViewTransitions.astro | 13 +------------ packages/astro/src/transitions/events.ts | 10 +++++----- packages/astro/src/transitions/router.ts | 8 ++++---- packages/astro/src/transitions/types.ts | 2 +- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 00376a57826f..310f1865a92c 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -117,18 +117,7 @@ const { fallback = 'animate' } = Astro.props; url.search = params.toString(); action = url.toString(); } else { - // Form elements without enctype explicitly set default to application/x-www-form-urlencoded. - // In order to maintain compatibility with Astro 4.x, we need to check the value of enctype - // on the attributes property rather than accessing .enctype directly. Astro 5.x may - // introduce defaulting to application/x-www-form-urlencoded as a breaking change, and then - // we can access .enctype directly. - // - // Note: getNamedItem can return null in real life, even if TypeScript doesn't think so. - const enctype = form.attributes.getNamedItem('enctype')?.value; - options.body = - enctype === 'application/x-www-form-urlencoded' - ? new URLSearchParams(formData as any) - : formData; + options.formData = formData; } ev.preventDefault(); diff --git a/packages/astro/src/transitions/events.ts b/packages/astro/src/transitions/events.ts index 691352d017f8..b3921b31f0c9 100644 --- a/packages/astro/src/transitions/events.ts +++ b/packages/astro/src/transitions/events.ts @@ -66,7 +66,7 @@ export const isTransitionBeforePreparationEvent = ( value: any ): value is TransitionBeforePreparationEvent => value.type === TRANSITION_BEFORE_PREPARATION; export class TransitionBeforePreparationEvent extends BeforeEvent { - body: FormData | URLSearchParams | undefined; + formData: FormData | undefined; loader: () => Promise; constructor( from: URL, @@ -76,7 +76,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent { sourceElement: Element | undefined, info: any, newDocument: Document, - body: FormData | URLSearchParams | undefined, + formData: FormData | undefined, loader: (event: TransitionBeforePreparationEvent) => Promise ) { super( @@ -90,7 +90,7 @@ export class TransitionBeforePreparationEvent extends BeforeEvent { info, newDocument ); - this.body = body; + this.formData = formData; this.loader = loader.bind(this, this); Object.defineProperties(this, { formData: { enumerable: true }, @@ -145,7 +145,7 @@ export async function doPreparation( navigationType: NavigationTypeString, sourceElement: Element | undefined, info: any, - body: FormData | URLSearchParams | undefined, + formData: FormData | undefined, defaultLoader: (event: TransitionBeforePreparationEvent) => Promise ) { const event = new TransitionBeforePreparationEvent( @@ -156,7 +156,7 @@ export async function doPreparation( sourceElement, info, window.document, - body, + formData, defaultLoader ); if (document.dispatchEvent(event)) { diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index ca160fa5d4c6..e710c2e1b2e8 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -449,7 +449,7 @@ async function transition( navigationType, options.sourceElement, options.info, - options.body, + options.formData, defaultLoader ); if (prepEvent.defaultPrevented) { @@ -460,9 +460,9 @@ async function transition( async function defaultLoader(preparationEvent: TransitionBeforePreparationEvent) { const href = preparationEvent.to.href; const init: RequestInit = {}; - if (preparationEvent.body) { + if (preparationEvent.formData) { init.method = 'POST'; - init.body = preparationEvent.body; + 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. @@ -488,7 +488,7 @@ async function transition( // 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.body + !preparationEvent.formData ) { preparationEvent.preventDefault(); return; diff --git a/packages/astro/src/transitions/types.ts b/packages/astro/src/transitions/types.ts index 9ad2ecdbbcf1..0e70825e5967 100644 --- a/packages/astro/src/transitions/types.ts +++ b/packages/astro/src/transitions/types.ts @@ -5,6 +5,6 @@ export type Options = { history?: 'auto' | 'push' | 'replace'; info?: any; state?: any; - body?: FormData | URLSearchParams; + formData?: FormData; sourceElement?: Element; // more than HTMLElement, e.g. SVGAElement }; From 0261b931a5a1c44367fbc58e24dd9af02fda843b Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 19 Dec 2023 10:40:56 -0500 Subject: [PATCH 4/8] Review feedback --- packages/astro/src/transitions/router.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index e710c2e1b2e8..ea45547131ce 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -462,7 +462,19 @@ async function transition( const init: RequestInit = {}; if (preparationEvent.formData) { init.method = 'POST'; - init.body = preparationEvent.formData; + // Form elements without enctype explicitly set default to application/x-www-form-urlencoded. + // In order to maintain compatibility with Astro 4.x, we need to check the value of enctype + // on the attributes property rather than accessing .enctype directly. Astro 5.x may + // introduce defaulting to application/x-www-form-urlencoded as a breaking change, and then + // we can access .enctype directly. + // + // Note: getNamedItem can return null in real life, even if TypeScript doesn't think so, hence + // the ?. + init.body = + preparationEvent.sourceElement?.attributes.getNamedItem('enctype')?.value === + 'application/x-www-form-urlencoded' + ? new URLSearchParams(preparationEvent.formData as any) + : preparationEvent.formData; } const response = await fetchHTML(href, init); // If there is a problem fetching the new page, just do an MPA navigation to it. From 6482ef74f74d1f96cad24087dca71670e894759e Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 19 Dec 2023 11:17:59 -0500 Subject: [PATCH 5/8] Handle submitter case --- packages/astro/src/transitions/router.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index ea45547131ce..afef08de5850 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -470,9 +470,15 @@ async function transition( // // Note: getNamedItem can return null in real life, even if TypeScript doesn't think so, hence // the ?. + const form = + preparationEvent.sourceElement instanceof HTMLFormElement + ? preparationEvent.sourceElement + : preparationEvent.sourceElement instanceof HTMLElement && + 'form' in preparationEvent.sourceElement + ? (preparationEvent.sourceElement.form as HTMLFormElement) + : preparationEvent.sourceElement?.closest('form'); init.body = - preparationEvent.sourceElement?.attributes.getNamedItem('enctype')?.value === - 'application/x-www-form-urlencoded' + form?.attributes.getNamedItem('enctype')?.value === 'application/x-www-form-urlencoded' ? new URLSearchParams(preparationEvent.formData as any) : preparationEvent.formData; } From 8f1f293b54910179bf7a4588c7cc20a891c72784 Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 19 Dec 2023 11:25:10 -0500 Subject: [PATCH 6/8] Move comment --- packages/astro/src/transitions/router.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/astro/src/transitions/router.ts b/packages/astro/src/transitions/router.ts index afef08de5850..e21a9daecb8d 100644 --- a/packages/astro/src/transitions/router.ts +++ b/packages/astro/src/transitions/router.ts @@ -462,6 +462,13 @@ async function transition( const init: RequestInit = {}; if (preparationEvent.formData) { init.method = 'POST'; + const form = + preparationEvent.sourceElement instanceof HTMLFormElement + ? preparationEvent.sourceElement + : preparationEvent.sourceElement instanceof HTMLElement && + 'form' in preparationEvent.sourceElement + ? (preparationEvent.sourceElement.form as HTMLFormElement) + : preparationEvent.sourceElement?.closest('form'); // Form elements without enctype explicitly set default to application/x-www-form-urlencoded. // In order to maintain compatibility with Astro 4.x, we need to check the value of enctype // on the attributes property rather than accessing .enctype directly. Astro 5.x may @@ -470,13 +477,6 @@ async function transition( // // Note: getNamedItem can return null in real life, even if TypeScript doesn't think so, hence // the ?. - const form = - preparationEvent.sourceElement instanceof HTMLFormElement - ? preparationEvent.sourceElement - : preparationEvent.sourceElement instanceof HTMLElement && - 'form' in preparationEvent.sourceElement - ? (preparationEvent.sourceElement.form as HTMLFormElement) - : preparationEvent.sourceElement?.closest('form'); init.body = form?.attributes.getNamedItem('enctype')?.value === 'application/x-www-form-urlencoded' ? new URLSearchParams(preparationEvent.formData as any) From de5ae2b4bf25bb58dbadfa4880902dd1cdc56425 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Tue, 19 Dec 2023 16:12:04 -0600 Subject: [PATCH 7/8] Update .changeset/rude-geckos-rush.md --- .changeset/rude-geckos-rush.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/rude-geckos-rush.md b/.changeset/rude-geckos-rush.md index 18fbdb831bf1..53f058151490 100644 --- a/.changeset/rude-geckos-rush.md +++ b/.changeset/rude-geckos-rush.md @@ -2,4 +2,4 @@ 'astro': patch --- -Respect forms with enctype set for view transitions +Updates view transitions `form` handling with logic for the [`enctype`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype) attribute From b3be36faf0c522cc67fa3d6fff2bf8e9e8dbd008 Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 19 Dec 2023 19:03:34 -0500 Subject: [PATCH 8/8] Add tests --- .../view-transitions/src/pages/form-one.astro | 10 ++- packages/astro/e2e/view-transitions.test.js | 77 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/form-one.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/form-one.astro index daa03b723d51..88a36251a72a 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/form-one.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/form-one.astro @@ -1,13 +1,15 @@ --- import Layout from '../components/Layout.astro'; const method = Astro.url.searchParams.get('method') ?? 'POST'; +const enctype = Astro.url.searchParams.get('enctype'); const postShowThrow = Astro.url.searchParams.has('throw') ?? false; --- +

Contact Form

-
- - {postShowThrow ? : ''} - + + + {postShowThrow ? : ''} +
diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index 222c9dfdf2aa..d402d72683ae 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -957,6 +957,83 @@ test.describe('View Transitions', () => { ).toEqual(1); }); + test('form POST defaults to multipart/form-data (Astro 4.x compatibility)', async ({ + page, + astro, + }) => { + const loads = []; + + page.addListener('load', async (p) => { + loads.push(p); + }); + + const postedEncodings = []; + + await page.route('**/contact', async (route) => { + const request = route.request(); + + if (request.method() === 'POST') { + postedEncodings.push(request.headers()['content-type'].split(';')[0]); + } + + await route.continue(); + }); + + await page.goto(astro.resolveUrl('/form-one')); + + // Submit the form + await page.click('#submit'); + + expect( + loads.length, + 'There should be only 1 page load. No additional loads for the form submission' + ).toEqual(1); + + expect( + postedEncodings, + 'There should be 1 POST, with encoding set to `multipart/form-data`' + ).toEqual(['multipart/form-data']); + }); + + test('form POST respects enctype attribute', async ({ page, astro }) => { + const loads = []; + + page.addListener('load', async (p) => { + loads.push(p); + }); + + const postedEncodings = []; + + await page.route('**/contact', async (route) => { + const request = route.request(); + + if (request.method() === 'POST') { + postedEncodings.push(request.headers()['content-type'].split(';')[0]); + } + + await route.continue(); + }); + + await page.goto( + astro.resolveUrl( + `/form-one?${new URLSearchParams({ enctype: 'application/x-www-form-urlencoded' })}` + ) + ); + + // Submit the form + await page.click('#submit'); + + expect( + loads.length, + 'There should be only 1 page load. No additional loads for the form submission' + ).toEqual(1); + + expect( + postedEncodings, + 'There should be 1 POST, with encoding set to `multipart/form-data`' + ).toEqual(['application/x-www-form-urlencoded']); + }); + test('Route announcer is invisible on page transition', async ({ page, astro }) => { await page.goto(astro.resolveUrl('/no-directive-one'));