Skip to content

feat: take form resets into account for two way bindings #10617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silly-ways-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: take form resets into account for two way bindings
Original file line number Diff line number Diff line change
Expand Up @@ -2850,6 +2850,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;
Expand Down
2 changes: 0 additions & 2 deletions packages/svelte/src/compiler/phases/bindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
41 changes: 31 additions & 10 deletions packages/svelte/src/internal/client/dom/elements/bindings/input.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { DEV } from 'esm-env';
import { render_effect } from '../../../reactivity/effects.js';
import { stringify } from '../../../render.js';
import { listen_to_event_and_reset_event } from './shared.js';

/**
* @param {HTMLInputElement} input
Expand All @@ -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'
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -114,7 +121,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);
});
Expand Down Expand Up @@ -168,3 +175,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();
});
}
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
);
}
}
Original file line number Diff line number Diff line change
@@ -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":""}`
);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<script>
let text = $state('text');
let checkbox = $state(true);
let radio_group = $state('a');
let checkbox_group = $state(['a']);
let select = $state('b');
let textarea = $state('textarea');
</script>

<p>{JSON.stringify({ text, checkbox, radio_group, checkbox_group, select, textarea })}</p>

<form>
<input bind:value={text} />

<input type="checkbox" bind:checked={checkbox} />

<input type="radio" name="radio" value="a" bind:group={radio_group} />
<input type="radio" name="radio" value="b" bind:group={radio_group} />

<input type="checkbox" name="checkbox" value="a" bind:group={checkbox_group} />
<input type="checkbox" name="checkbox" value="b" bind:group={checkbox_group} />

<select bind:value={select}>
<option value="a">a</option>
<option value="b">b</option>
</select>

<textarea bind:value={textarea}></textarea>

<button type="button" onclick={(e) => e.target.form.reset()}>Reset</button>
</form>
Original file line number Diff line number Diff line change
Expand Up @@ -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.