From 3eef1cb8cf5673b4326d53961050a6c8e59bf81c Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Fri, 22 Mar 2024 18:40:04 +0100
Subject: [PATCH] feat: take form resets into account for two way bindings
(#10617)
* feat: take form resets into account for two way bindings
When resetting a form, the value of the inputs within it get out of sync with the bound value of those inputs. This PR introduces a reset listener on the parent form to reset the value in that case
closes #2659
* slightly different approach
* tweaks, test
* this is a breaking change, strictly speaking
* bind:files
* use capture phase
* tweak wording
* use promise, explain
---
.changeset/silly-ways-wash.md | 5 ++
.../3-transform/client/visitors/template.js | 4 ++
.../svelte/src/compiler/phases/bindings.js | 2 -
.../client/dom/elements/bindings/input.js | 41 ++++++++++++----
.../client/dom/elements/bindings/select.js | 3 +-
.../client/dom/elements/bindings/shared.js | 48 +++++++++++++++++++
.../samples/bindings-form-reset/_config.js | 19 ++++++++
.../samples/bindings-form-reset/main.svelte | 31 ++++++++++++
.../03-appendix/02-breaking-changes.md | 4 ++
9 files changed, 144 insertions(+), 13 deletions(-)
create mode 100644 .changeset/silly-ways-wash.md
create mode 100644 packages/svelte/tests/runtime-runes/samples/bindings-form-reset/_config.js
create mode 100644 packages/svelte/tests/runtime-runes/samples/bindings-form-reset/main.svelte
diff --git a/.changeset/silly-ways-wash.md b/.changeset/silly-ways-wash.md
new file mode 100644
index 0000000000..5157693e80
--- /dev/null
+++ b/.changeset/silly-ways-wash.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+feat: take form resets into account for two way bindings
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
index 7e4a82bef9..01daedb26c 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
@@ -2892,6 +2892,10 @@ export const template_visitors = {
break;
}
+ case 'files':
+ call_expr = b.call(`$.bind_files`, state.node, getter, setter);
+ break;
+
case 'this':
call_expr = serialize_bind_this(node.expression, context, state.node);
break;
diff --git a/packages/svelte/src/compiler/phases/bindings.js b/packages/svelte/src/compiler/phases/bindings.js
index 820c66d886..e3fdd7eb8c 100644
--- a/packages/svelte/src/compiler/phases/bindings.js
+++ b/packages/svelte/src/compiler/phases/bindings.js
@@ -182,8 +182,6 @@ export const binding_properties = {
valid_elements: ['input', 'textarea', 'select']
},
files: {
- event: 'change',
- type: 'set',
valid_elements: ['input'],
omit_in_ssr: true
}
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js
index 86d75948ee..ce399aa228 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js
@@ -1,6 +1,7 @@
import { DEV } from 'esm-env';
import { render_effect, user_effect } from '../../../reactivity/effects.js';
import { stringify } from '../../../render.js';
+import { listen_to_event_and_reset_event } from './shared.js';
/**
* @param {HTMLInputElement} input
@@ -9,7 +10,7 @@ import { stringify } from '../../../render.js';
* @returns {void}
*/
export function bind_value(input, get_value, update) {
- input.addEventListener('input', () => {
+ listen_to_event_and_reset_event(input, 'input', () => {
if (DEV && input.type === 'checkbox') {
throw new Error(
'Using bind:value together with a checkbox input is not allowed. Use bind:checked instead'
@@ -72,16 +73,22 @@ export function bind_group(inputs, group_index, input, get_value, update) {
binding_group.push(input);
- input.addEventListener('change', () => {
- // @ts-ignore
- var value = input.__value;
+ listen_to_event_and_reset_event(
+ input,
+ 'change',
+ () => {
+ // @ts-ignore
+ var value = input.__value;
- if (is_checkbox) {
- value = get_binding_group_value(binding_group, value, input.checked);
- }
+ if (is_checkbox) {
+ value = get_binding_group_value(binding_group, value, input.checked);
+ }
- update(value);
- });
+ update(value);
+ },
+ // TODO better default value handling
+ () => update(is_checkbox ? [] : null)
+ );
render_effect(() => {
var value = get_value();
@@ -119,7 +126,7 @@ export function bind_group(inputs, group_index, input, get_value, update) {
* @returns {void}
*/
export function bind_checked(input, get_value, update) {
- input.addEventListener('change', () => {
+ listen_to_event_and_reset_event(input, 'change', () => {
var value = input.checked;
update(value);
});
@@ -173,3 +180,17 @@ function is_numberlike_input(input) {
function to_number(value) {
return value === '' ? null : +value;
}
+
+/**
+ * @param {HTMLInputElement} input
+ * @param {() => FileList | null} get_value
+ * @param {(value: FileList | null) => void} update
+ */
+export function bind_files(input, get_value, update) {
+ listen_to_event_and_reset_event(input, 'change', () => {
+ update(input.files);
+ });
+ render_effect(() => {
+ input.files = get_value();
+ });
+}
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js
index 6ee497f02b..840be3a7af 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js
@@ -1,4 +1,5 @@
import { effect } from '../../../reactivity/effects.js';
+import { listen_to_event_and_reset_event } from './shared.js';
import { untrack } from '../../../runtime.js';
/**
@@ -76,7 +77,7 @@ export function init_select(select, get_value) {
export function bind_select_value(select, get_value, update) {
var mounting = true;
- select.addEventListener('change', () => {
+ listen_to_event_and_reset_event(select, 'change', () => {
/** @type {unknown} */
var value;
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/shared.js b/packages/svelte/src/internal/client/dom/elements/bindings/shared.js
index 379ce82f1e..3c4f55259a 100644
--- a/packages/svelte/src/internal/client/dom/elements/bindings/shared.js
+++ b/packages/svelte/src/internal/client/dom/elements/bindings/shared.js
@@ -25,3 +25,51 @@ export function listen(target, events, handler, call_handler_immediately = true)
};
});
}
+
+let listening_to_form_reset = false;
+
+/**
+ * Listen to the given event, and then instantiate a global form reset listener if not already done,
+ * to notify all bindings when the form is reset
+ * @param {HTMLElement} element
+ * @param {string} event
+ * @param {() => void} handler
+ * @param {() => void} [on_reset]
+ */
+export function listen_to_event_and_reset_event(element, event, handler, on_reset = handler) {
+ element.addEventListener(event, handler);
+ // @ts-expect-error
+ const prev = element.__on_r;
+ if (prev) {
+ // special case for checkbox that can have multiple binds (group & checked)
+ // @ts-expect-error
+ element.__on_r = () => {
+ prev();
+ on_reset();
+ };
+ } else {
+ // @ts-expect-error
+ element.__on_r = on_reset;
+ }
+
+ if (!listening_to_form_reset) {
+ listening_to_form_reset = true;
+ document.addEventListener(
+ 'reset',
+ (evt) => {
+ // Needs to happen one tick later or else the dom properties of the form
+ // elements have not updated to their reset values yet
+ Promise.resolve().then(() => {
+ if (!evt.defaultPrevented) {
+ for (const e of /**@type {HTMLFormElement} */ (evt.target).elements) {
+ // @ts-expect-error
+ e.__on_r?.();
+ }
+ }
+ });
+ },
+ // In the capture phase to guarantee we get noticed of it (no possiblity of stopPropagation)
+ { capture: true }
+ );
+ }
+}
diff --git a/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/_config.js b/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/_config.js
new file mode 100644
index 0000000000..069d183177
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/_config.js
@@ -0,0 +1,19 @@
+import { test } from '../../test';
+
+export default test({
+ async test({ assert, target }) {
+ const p = target.querySelector('p');
+
+ assert.htmlEqual(
+ p?.innerHTML || '',
+ `{"text":"text","checkbox":true,"radio_group":"a","checkbox_group":["a"],"select":"b","textarea":"textarea"}`
+ );
+
+ await target.querySelector('button')?.click();
+ await Promise.resolve();
+ assert.htmlEqual(
+ p?.innerHTML || '',
+ `{"text":"","checkbox":false,"radio_group":null,"checkbox_group":[],"select":"a","textarea":""}`
+ );
+ }
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/main.svelte b/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/main.svelte
new file mode 100644
index 0000000000..ff13af85c8
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/bindings-form-reset/main.svelte
@@ -0,0 +1,31 @@
+
+
+{JSON.stringify({ text, checkbox, radio_group, checkbox_group, select, textarea })}
+
+
diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md
index 205bdd2bf2..d88a749bc8 100644
--- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md
+++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md
@@ -179,3 +179,7 @@ In Svelte 4, `null` and `undefined` were printed as the corresponding string. In
### `bind:files` values can only be `null`, `undefined` or `FileList`
`bind:files` is now a two-way binding. As such, when setting a value, it needs to be either falsy (`null` or `undefined`) or of type `FileList`.
+
+### Bindings now react to form resets
+
+Previously, bindings did not take into account `reset` event of forms, and therefore values could get out of sync with the DOM. Svelte 5 fixes this by placing a `reset` listener on the document and invoking bindings where necessary.