From 6eb9b3d0f87de08f0852fb9089c5e8ef7fc4ddca Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 9 May 2023 17:39:15 -0600 Subject: [PATCH 01/11] fix: Package name keeps me from filtering with pnpm --- packages/kit/test/apps/dev-only/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/dev-only/package.json b/packages/kit/test/apps/dev-only/package.json index 37ee1ab3a73d..34c9b1b80012 100644 --- a/packages/kit/test/apps/dev-only/package.json +++ b/packages/kit/test/apps/dev-only/package.json @@ -1,5 +1,5 @@ { - "name": "test-basics", + "name": "test-dev-only", "private": true, "version": "0.0.2-next.0", "scripts": { From 3adbfb0a920257fa5e232dd0b4361c9f0633173b Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 9 May 2023 17:41:54 -0600 Subject: [PATCH 02/11] feat: Warn users when submitting a form containing a file without the correct enctype --- packages/kit/src/runtime/app/forms.js | 11 +++++++++++ .../actions/file-without-enctype/+page.server.js | 6 ++++++ .../actions/file-without-enctype/+page.svelte | 8 ++++++++ packages/kit/test/apps/basics/test/test.js | 14 ++++++++++++++ 4 files changed, 39 insertions(+) create mode 100644 packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.svelte diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index 24ef54daf79d..9640ad2ea35a 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -65,6 +65,17 @@ export function enhance(form, submit = () => {}) { const data = new FormData(form); + if (DEV && form.enctype !== 'multipart/form-data') { + for (const value of data.values()) { + if (value instanceof File) { + // TODO 2.0: Upgrade to `throw Error` + console.warn( + 'Your form contains fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.' + ); + } + } + } + const submitter_name = event.submitter?.getAttribute('name'); if (submitter_name) { data.append(submitter_name, event.submitter?.getAttribute('value') ?? ''); diff --git a/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.server.js new file mode 100644 index 000000000000..05260b09a0eb --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.server.js @@ -0,0 +1,6 @@ +export const actions = { + default: async ({ request }) => { + const data = await request.formData(); + console.log(data.get('file')); + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.svelte new file mode 100644 index 000000000000..c33bef9f7e21 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/file-without-enctype/+page.svelte @@ -0,0 +1,8 @@ + + +
+ + +
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6b976d2eee5d..20e39c2c5717 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -828,6 +828,20 @@ test.describe('Matchers', () => { }); test.describe('Actions', () => { + test('Submitting a form with a file input but no enctype="multipart/form-data" logs a warning', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled || !process.env.DEV) return; + await page.goto('/actions/file-without-enctype'); + const logPromise = page.waitForEvent('console'); + await page.click('button'); + const log = await logPromise; + expect(log.text()).toBe( + 'Your form contains fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.' + ); + }); + test('Error props are returned', async ({ page, javaScriptEnabled }) => { await page.goto('/actions/form-errors'); await page.click('button'); From 29f19126cbbf4193c5b6af5f32c4c951b5567985 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 9 May 2023 17:44:04 -0600 Subject: [PATCH 03/11] changeset --- .changeset/tasty-llamas-relate.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tasty-llamas-relate.md diff --git a/.changeset/tasty-llamas-relate.md b/.changeset/tasty-llamas-relate.md new file mode 100644 index 000000000000..ae11fbefe358 --- /dev/null +++ b/.changeset/tasty-llamas-relate.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +[feat] Warn users when enhancing forms with files but no `enctype="multipart/form-data"` From 9296e1c5bb6d89267a92c3ca8e2d5509dfa0e759 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 9 May 2023 19:12:18 -0600 Subject: [PATCH 04/11] Update packages/kit/src/runtime/app/forms.js Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> --- packages/kit/src/runtime/app/forms.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index 9640ad2ea35a..a74e0df96615 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -65,7 +65,7 @@ export function enhance(form, submit = () => {}) { const data = new FormData(form); - if (DEV && form.enctype !== 'multipart/form-data') { + if (DEV && /** @type {HTMLFormElement} */ (HTMLElement.prototype.cloneNode.call(form)).enctype !== 'multipart/form-data') { for (const value of data.values()) { if (value instanceof File) { // TODO 2.0: Upgrade to `throw Error` From b95dca670e50b219a959aa96bd06e120ec37e9e0 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Tue, 9 May 2023 21:17:47 -0600 Subject: [PATCH 05/11] style --- packages/kit/src/runtime/app/forms.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index a74e0df96615..29701ff06237 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -65,7 +65,11 @@ export function enhance(form, submit = () => {}) { const data = new FormData(form); - if (DEV && /** @type {HTMLFormElement} */ (HTMLElement.prototype.cloneNode.call(form)).enctype !== 'multipart/form-data') { + if ( + DEV && + /** @type {HTMLFormElement} */ (HTMLElement.prototype.cloneNode.call(form)).enctype !== + 'multipart/form-data' + ) { for (const value of data.values()) { if (value instanceof File) { // TODO 2.0: Upgrade to `throw Error` From 93fda801220dbed998fffaafd418e994db640fca Mon Sep 17 00:00:00 2001 From: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> Date: Wed, 10 May 2023 11:54:21 +0545 Subject: [PATCH 06/11] moar style tweaks --- .changeset/tasty-llamas-relate.md | 2 +- packages/kit/test/apps/basics/test/test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/tasty-llamas-relate.md b/.changeset/tasty-llamas-relate.md index ae11fbefe358..8ebc81eeec75 100644 --- a/.changeset/tasty-llamas-relate.md +++ b/.changeset/tasty-llamas-relate.md @@ -2,4 +2,4 @@ '@sveltejs/kit': minor --- -[feat] Warn users when enhancing forms with files but no `enctype="multipart/form-data"` +feat: Warn users when enhancing forms with files but no `enctype="multipart/form-data"` diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 20e39c2c5717..efaac4aad134 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -834,9 +834,9 @@ test.describe('Actions', () => { }) => { if (!javaScriptEnabled || !process.env.DEV) return; await page.goto('/actions/file-without-enctype'); - const logPromise = page.waitForEvent('console'); + const log_promise = page.waitForEvent('console'); await page.click('button'); - const log = await logPromise; + const log = await log_promise; expect(log.text()).toBe( 'Your form contains fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.' ); From 6cf0e588d12eb03935d10cfbc57b7228c4fd4d78 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Wed, 10 May 2023 21:10:27 -0600 Subject: [PATCH 07/11] better test skip --- packages/kit/test/apps/basics/test/test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 20e39c2c5717..bd510be1b7ea 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -832,7 +832,8 @@ test.describe('Actions', () => { page, javaScriptEnabled }) => { - if (!javaScriptEnabled || !process.env.DEV) return; + test.skip(!javaScriptEnabled, 'Skip when JavaScript is disabled'); + test.skip(!process.env.DEV, 'Skip when not in dev mode'); await page.goto('/actions/file-without-enctype'); const logPromise = page.waitForEvent('console'); await page.click('button'); From 6522e370a75bc02f2ebf69f91a045016d65160c0 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Wed, 10 May 2023 21:13:30 -0600 Subject: [PATCH 08/11] Update .changeset/tasty-llamas-relate.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- .changeset/tasty-llamas-relate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tasty-llamas-relate.md b/.changeset/tasty-llamas-relate.md index 8ebc81eeec75..b7853026ce61 100644 --- a/.changeset/tasty-llamas-relate.md +++ b/.changeset/tasty-llamas-relate.md @@ -2,4 +2,4 @@ '@sveltejs/kit': minor --- -feat: Warn users when enhancing forms with files but no `enctype="multipart/form-data"` +feat: warn users when enhancing forms with files but no `enctype="multipart/form-data"` From 601aee4c9749828f4cab635f610a365647788c69 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 16 May 2023 18:17:27 -0400 Subject: [PATCH 09/11] DRY out --- packages/kit/src/runtime/app/forms.js | 29 ++++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index ab9186d2e1e4..181fe4ac5246 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -28,13 +28,20 @@ function warn_on_access(old_name, new_name, call_location) { ); } +/** + * Shallow clone an element, so that we can access e.g. `form.action` without worrying + * that someone has added an `` (https://github.com/sveltejs/kit/issues/7593) + * @template {HTMLElement} T + * @param {T} element + * @returns {T} + */ +function clone(element) { + return /** @type {T} */ (HTMLElement.prototype.cloneNode.call(element)); +} + /** @type {import('$app/forms').enhance} */ export function enhance(form_element, submit = () => {}) { - if ( - DEV && - /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element)) - .method !== 'post' - ) { + if (DEV && clone(form_element).method !== 'post') { throw new Error('use:enhance can only be used on
fields with method="POST"'); } @@ -71,21 +78,15 @@ export function enhance(form_element, submit = () => {}) { const action = new URL( // We can't do submitter.formAction directly because that property is always set - // We do cloneNode for avoid DOM clobbering - https://github.com/sveltejs/kit/issues/7593 event.submitter?.hasAttribute('formaction') ? /** @type {HTMLButtonElement | HTMLInputElement} */ (event.submitter).formAction - : /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element)) - .action + : clone(form_element).action ); const form_data = new FormData(form_element); - if ( - DEV && - /** @type {HTMLFormElement} */ (HTMLElement.prototype.cloneNode.call(form)).enctype !== - 'multipart/form-data' - ) { - for (const value of data.values()) { + if (DEV && clone(form_element).enctype !== 'multipart/form-data') { + for (const value of form_data.values()) { if (value instanceof File) { // TODO 2.0: Upgrade to `throw Error` console.warn( From 79ac2dbdebe55290885b23cccc33b5aa6ff20dca Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 16 May 2023 18:17:40 -0400 Subject: [PATCH 10/11] only warn once per submit --- packages/kit/src/runtime/app/forms.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index 181fe4ac5246..30858235032f 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -92,6 +92,7 @@ export function enhance(form_element, submit = () => {}) { console.warn( 'Your form contains fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.' ); + break; } } } From b48d58e77a8f807075f875f1f1aea420ccec3760 Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Wed, 17 May 2023 10:42:28 -0600 Subject: [PATCH 11/11] Update .changeset/tasty-llamas-relate.md Co-authored-by: Rich Harris --- .changeset/tasty-llamas-relate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tasty-llamas-relate.md b/.changeset/tasty-llamas-relate.md index b7853026ce61..20637dced526 100644 --- a/.changeset/tasty-llamas-relate.md +++ b/.changeset/tasty-llamas-relate.md @@ -1,5 +1,5 @@ --- -'@sveltejs/kit': minor +'@sveltejs/kit': patch --- feat: warn users when enhancing forms with files but no `enctype="multipart/form-data"`