diff --git a/.changeset/odd-crews-own.md b/.changeset/odd-crews-own.md new file mode 100644 index 000000000000..1f140b9b965b --- /dev/null +++ b/.changeset/odd-crews-own.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +feat: unshadow `data` and `form` in `enhance` and warn about future deprecation when used in `dev` mode diff --git a/packages/kit/src/runtime/app/forms.js b/packages/kit/src/runtime/app/forms.js index 24ef54daf79d..4a5a9abfbe0a 100644 --- a/packages/kit/src/runtime/app/forms.js +++ b/packages/kit/src/runtime/app/forms.js @@ -14,12 +14,26 @@ export function deserialize(result) { return parsed; } +/** + * @param {string} old_name + * @param {string} new_name + * @param {string} call_location + * @returns void + */ +function warn_on_access(old_name, new_name, call_location) { + if (!DEV) return; + // TODO 2.0: Remove this code + console.warn( + `\`${old_name}\` has been deprecated in favor of \`${new_name}\`. \`${old_name}\` will be removed in a future version. (Called from ${call_location})` + ); +} + /** @type {import('$app/forms').enhance} */ -export function enhance(form, submit = () => {}) { +export function enhance(form_element, submit = () => {}) { if ( DEV && - /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form)).method !== - 'post' + /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element)) + .method !== 'post' ) { throw new Error('use:enhance can only be used on
fields with method="POST"'); } @@ -35,7 +49,7 @@ export function enhance(form, submit = () => {}) { if (result.type === 'success') { if (reset !== false) { // We call reset from the prototype to avoid DOM clobbering - HTMLFormElement.prototype.reset.call(form); + HTMLFormElement.prototype.reset.call(form_element); } await invalidateAll(); } @@ -60,14 +74,15 @@ export function enhance(form, submit = () => {}) { // 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)).action + : /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element)) + .action ); - const data = new FormData(form); + const form_data = new FormData(form_element); const submitter_name = event.submitter?.getAttribute('name'); if (submitter_name) { - data.append(submitter_name, event.submitter?.getAttribute('value') ?? ''); + form_data.append(submitter_name, event.submitter?.getAttribute('value') ?? ''); } const controller = new AbortController(); @@ -75,13 +90,22 @@ export function enhance(form, submit = () => {}) { let cancelled = false; const cancel = () => (cancelled = true); + // TODO 2.0: Remove `data` and `form` const callback = (await submit({ action, cancel, controller, - data, - form, + get data() { + warn_on_access('data', 'formData', 'use:enhance submit function'); + return form_data; + }, + formData: form_data, + get form() { + warn_on_access('form', 'formElement', 'use:enhance submit function'); + return form_element; + }, + formElement: form_element, submitter: event.submitter })) ?? fallback_callback; if (cancelled) return; @@ -97,7 +121,7 @@ export function enhance(form, submit = () => {}) { 'x-sveltekit-action': 'true' }, cache: 'no-store', - body: data, + body: form_data, signal: controller.signal }); @@ -110,8 +134,16 @@ export function enhance(form, submit = () => {}) { callback({ action, - data, - form, + get data() { + warn_on_access('data', 'formData', 'callback returned from use:enhance submit function'); + return form_data; + }, + formData: form_data, + get form() { + warn_on_access('form', 'formElement', 'callback returned from use:enhance submit function'); + return form_element; + }, + formElement: form_element, update: (opts) => fallback_callback({ action, result, reset: opts?.reset }), // @ts-expect-error generic constraints stuff we don't care about result @@ -119,12 +151,12 @@ export function enhance(form, submit = () => {}) { } // @ts-expect-error - HTMLFormElement.prototype.addEventListener.call(form, 'submit', handle_submit); + HTMLFormElement.prototype.addEventListener.call(form_element, 'submit', handle_submit); return { destroy() { // @ts-expect-error - HTMLFormElement.prototype.removeEventListener.call(form, 'submit', handle_submit); + HTMLFormElement.prototype.removeEventListener.call(form_element, 'submit', handle_submit); } }; } diff --git a/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.server.js b/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.server.js new file mode 100644 index 000000000000..4bdf1c67412b --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.server.js @@ -0,0 +1,26 @@ +// TODO 2.0: Remove this code and corresponding tests +export const actions = { + form_submit: () => { + return { + form_submit: true + }; + }, + + form_callback: () => { + return { + form_callback: true + }; + }, + + data_submit: () => { + return { + data_submit: true + }; + }, + + data_callback: () => { + return { + data_callback: true + }; + } +}; diff --git a/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.svelte b/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.svelte new file mode 100644 index 000000000000..290d6d57670d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/actions/enhance/old-property-access/+page.svelte @@ -0,0 +1,52 @@ + + + + +
+ +
+ +
+ +
+ +
+ +
+ +
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6b976d2eee5d..1571911d7e7b 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -828,6 +828,51 @@ test.describe('Matchers', () => { }); test.describe('Actions', () => { + for (const { id, old_name, new_name, call_location } of [ + { + id: 'access-form-in-submit', + old_name: 'form', + new_name: 'formElement', + call_location: 'use:enhance submit function' + }, + { + id: 'access-form-in-callback', + old_name: 'form', + new_name: 'formElement', + call_location: 'callback returned from use:enhance submit function' + }, + { + id: 'access-data-in-submit', + old_name: 'data', + new_name: 'formData', + call_location: 'use:enhance submit function' + }, + { + id: 'access-data-in-callback', + old_name: 'data', + new_name: 'formData', + call_location: 'callback returned from use:enhance submit function' + } + ]) { + test(`Accessing v2 deprecated properties results in a warning log, type: ${id}`, async ({ + page, + javaScriptEnabled + }) => { + test.skip(!javaScriptEnabled, 'skip when js is disabled'); + test.skip(!process.env.DEV, 'skip when not in dev mode'); + await page.goto('/actions/enhance/old-property-access'); + const log_promise = page.waitForEvent('console'); + const button = page.locator(`#${id}`); + await button.click(); + expect(await button.textContent()).toBe('processed'); // needed to make sure action completes + const log = await log_promise; + expect(log.text()).toBe( + `\`${old_name}\` has been deprecated in favor of \`${new_name}\`. \`${old_name}\` will be removed in a future version. (Called from ${call_location})` + ); + expect(log.type()).toBe('warning'); + }); + } + test('Error props are returned', async ({ page, javaScriptEnabled }) => { await page.goto('/actions/form-errors'); await page.click('button'); 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": { diff --git a/packages/kit/types/ambient.d.ts b/packages/kit/types/ambient.d.ts index 9e6b92fa27e0..10e3529d238b 100644 --- a/packages/kit/types/ambient.d.ts +++ b/packages/kit/types/ambient.d.ts @@ -80,15 +80,36 @@ declare module '$app/forms' { Invalid extends Record | undefined = Record > = (input: { action: URL; + /** + * use `formData` instead of `data` + * @deprecated + */ data: FormData; + formData: FormData; + /** + * use `formElement` instead of `form` + * @deprecated + */ form: HTMLFormElement; + formElement: HTMLFormElement; controller: AbortController; cancel(): void; submitter: HTMLElement | null; }) => MaybePromise< | void | ((opts: { + /** + * use `formData` instead of `data` + * @deprecated + */ + data: FormData; + formData: FormData; + /** + * use `formElement` instead of `form` + * @deprecated + */ form: HTMLFormElement; + formElement: HTMLFormElement; action: URL; result: ActionResult; /** @@ -108,7 +129,7 @@ declare module '$app/forms' { Success extends Record | undefined = Record, Invalid extends Record | undefined = Record >( - form: HTMLFormElement, + formElement: HTMLFormElement, /** * Called upon submission with the given FormData and the `action` that should be triggered. * If `cancel` is called, the form will not be submitted.