Skip to content

Commit

Permalink
feat: unshadow form and data in enhance (#9902)
Browse files Browse the repository at this point in the history
* feat: Un-shadow `data` and `form` in `enhance`, warn about future deprecation in dev

* changeset

* snek

* Update .changeset/odd-crews-own.md

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update packages/kit/test/apps/dev-only/package.json

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* am not smart

* still not smart

* oops

* oof

* add deprecation notice

---------

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <git@rich-harris.dev>
  • Loading branch information
3 people authored May 16, 2023
1 parent 7042766 commit 4a85b7f
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-crews-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': minor
---

feat: unshadow `data` and `form` in `enhance` and warn about future deprecation when used in `dev` mode
60 changes: 46 additions & 14 deletions packages/kit/src/runtime/app/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <form> fields with method="POST"');
}
Expand All @@ -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();
}
Expand All @@ -60,28 +74,38 @@ 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();

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;
Expand All @@ -97,7 +121,7 @@ export function enhance(form, submit = () => {}) {
'x-sveltekit-action': 'true'
},
cache: 'no-store',
body: data,
body: form_data,
signal: controller.signal
});

Expand All @@ -110,21 +134,29 @@ 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
});
}

// @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);
}
};
}
Original file line number Diff line number Diff line change
@@ -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
};
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<script>
import { enhance } from '$app/forms';
export let form;
const access_form_submit = (node) => {
return enhance(node, ({ form }) => {});
};
const access_data_submit = (node) => {
return enhance(node, ({ data }) => {});
};
const access_form_callback = (node) => {
return enhance(
node,
() =>
({ form, update }) =>
update()
);
};
const access_data_callback = (node) => {
return enhance(
node,
() =>
({ data, update }) =>
update()
);
};
</script>

<form method="POST" action="?/form_submit" use:access_form_submit>
<button id="access-form-in-submit" type="submit"
>{form?.form_submit ? 'processed' : 'not processed'}</button
>
</form>

<form method="POST" action="?/data_submit" use:access_data_submit>
<button id="access-data-in-submit" type="submit"
>{form?.data_submit ? 'processed' : 'not processed'}</button
>
</form>

<form method="POST" action="?/form_callback" use:access_form_callback>
<button id="access-form-in-callback" type="submit"
>{form?.form_callback ? 'processed' : 'not processed'}</button
>
</form>

<form method="POST" action="?/data_callback" use:access_data_callback>
<button id="access-data-in-callback" type="submit"
>{form?.data_callback ? 'processed' : 'not processed'}</button
>
</form>
45 changes: 45 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/dev-only/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "test-basics",
"name": "test-dev-only",
"private": true,
"version": "0.0.2-next.0",
"scripts": {
Expand Down
23 changes: 22 additions & 1 deletion packages/kit/types/ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,36 @@ declare module '$app/forms' {
Invalid extends Record<string, unknown> | undefined = Record<string, any>
> = (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<Success, Invalid>;
/**
Expand All @@ -108,7 +129,7 @@ declare module '$app/forms' {
Success extends Record<string, unknown> | undefined = Record<string, any>,
Invalid extends Record<string, unknown> | undefined = Record<string, any>
>(
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.
Expand Down

0 comments on commit 4a85b7f

Please sign in to comment.