From 0d51dbae3205700516e13840b02e4fd7a9c2576f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 11 Jun 2024 14:19:22 +0100 Subject: [PATCH] fix: ensure bound input content is resumed on hydration (#11986) * fix: ensure bound input content is resumed on hydration * fix: ensure bound input content is resumed on hydration * Update packages/svelte/src/internal/client/dom/elements/bindings/input.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * fix: ensure bound input content is resumed on hydration * fix: ensure bound input content is resumed on hydration * Update packages/svelte/src/internal/client/dom/elements/bindings/input.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * add test * add test * add test * add test * Update packages/svelte/src/internal/client/dom/elements/bindings/input.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * add test * add test * newlines between multi-line blocks, let the code breathe --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Rich Harris --- .changeset/flat-feet-visit.md | 5 +++ .../client/dom/elements/bindings/input.js | 33 ++++++++++++++- .../svelte/tests/runtime-legacy/shared.ts | 42 +++++++++++++++---- .../hydrate-modified-input-group/_config.js | 18 ++++++++ .../hydrate-modified-input-group/main.svelte | 9 ++++ .../samples/hydrate-modified-input/_config.js | 15 +++++++ .../hydrate-modified-input/main.svelte | 6 +++ playgrounds/demo/src/entry-client.ts | 13 +++--- 8 files changed, 125 insertions(+), 16 deletions(-) create mode 100644 .changeset/flat-feet-visit.md create mode 100644 packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/main.svelte diff --git a/.changeset/flat-feet-visit.md b/.changeset/flat-feet-visit.md new file mode 100644 index 0000000000..0d99c89318 --- /dev/null +++ b/.changeset/flat-feet-visit.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: ensure bound input content is resumed on hydration 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 3ccbb08c72..d9ec022dd0 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -4,6 +4,7 @@ import { listen_to_event_and_reset_event } from './shared.js'; import * as e from '../../../errors.js'; import { get_proxied_value, is } from '../../../proxy.js'; import { queue_micro_task } from '../../task.js'; +import { hydrating } from '../../hydration.js'; /** * @param {HTMLInputElement} input @@ -29,8 +30,12 @@ export function bind_value(input, get_value, update) { var value = get_value(); - // @ts-ignore - input.__value = value; + // If we are hydrating and the value has since changed, then use the update value + // from the input instead. + if (hydrating && input.defaultValue !== input.value) { + update(input.value); + return; + } if (is_numberlike_input(input) && value === to_number(input.value)) { // handles 0 vs 00 case (see https://github.com/sveltejs/svelte/issues/9959) @@ -60,6 +65,9 @@ export function bind_group(inputs, group_index, input, get_value, update) { var is_checkbox = input.getAttribute('type') === 'checkbox'; var binding_group = inputs; + // needs to be let or related code isn't treeshaken out if it's always false + let hydration_mismatch = false; + if (group_index !== null) { for (var index of group_index) { var group = binding_group; @@ -94,6 +102,13 @@ export function bind_group(inputs, group_index, input, get_value, update) { render_effect(() => { var value = get_value(); + // If we are hydrating and the value has since changed, then use the update value + // from the input instead. + if (hydrating && input.defaultChecked !== input.checked) { + hydration_mismatch = true; + return; + } + if (is_checkbox) { value = value || []; // @ts-ignore @@ -115,6 +130,20 @@ export function bind_group(inputs, group_index, input, get_value, update) { queue_micro_task(() => { // necessary to maintain binding group order in all insertion scenarios. TODO optimise binding_group.sort((a, b) => (a.compareDocumentPosition(b) === 4 ? -1 : 1)); + + if (hydration_mismatch) { + var value; + + if (is_checkbox) { + value = get_binding_group_value(binding_group, value, input.checked); + } else { + var hydration_input = binding_group.find((input) => input.checked); + // @ts-ignore + value = hydration_input?.__value; + } + + update(value); + } }); } diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index 8d3ea09e45..d7b64302b7 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -59,6 +59,7 @@ export interface RuntimeTest = Record void | Promise; test_ssr?: (args: { assert: Assert }) => void | Promise; accessors?: boolean; @@ -103,6 +104,10 @@ export function runtime_suite(runes: boolean) { if (config.skip_mode?.includes('hydrate')) return true; } + if (variant === 'dom' && config.skip_mode?.includes('client')) { + return 'no-test'; + } + if (variant === 'ssr') { if ( (config.mode && !config.mode.includes('server')) || @@ -161,6 +166,7 @@ async function run_test_variant( let logs: string[] = []; let warnings: string[] = []; + let manual_hydrate = false; { // use some crude static analysis to determine if logs/warnings are intercepted. @@ -180,6 +186,10 @@ async function run_test_variant( console.log = (...args) => logs.push(...args); } + if (str.slice(0, i).includes('hydrate')) { + manual_hydrate = true; + } + if (str.slice(0, i).includes('warnings') || config.warnings) { // eslint-disable-next-line no-console console.warn = (...args) => { @@ -297,17 +307,30 @@ async function run_test_variant( let instance: any; let props: any; + let hydrate_fn: Function = () => { + throw new Error('Ensure dom mode is skipped'); + }; if (runes) { props = proxy({ ...(config.props || {}) }); - - const render = variant === 'hydrate' ? hydrate : mount; - instance = render(mod.default, { - target, - props, - intro: config.intro, - recover: config.recover ?? false - }); + if (manual_hydrate) { + hydrate_fn = () => { + instance = hydrate(mod.default, { + target, + props, + intro: config.intro, + recover: config.recover ?? false + }); + }; + } else { + const render = variant === 'hydrate' ? hydrate : mount; + instance = render(mod.default, { + target, + props, + intro: config.intro, + recover: config.recover ?? false + }); + } } else { instance = createClassComponent({ component: mod.default, @@ -357,7 +380,8 @@ async function run_test_variant( raf, compileOptions, logs, - warnings + warnings, + hydrate: hydrate_fn }); } diff --git a/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/_config.js b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/_config.js new file mode 100644 index 0000000000..7f3bfac707 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/_config.js @@ -0,0 +1,18 @@ +import { test } from '../../test'; + +export default test({ + skip_mode: ['client'], + + test({ assert, target, hydrate }) { + const inputs = /** @type {NodeListOf} */ (target.querySelectorAll('input')); + inputs[1].checked = true; + inputs[1].dispatchEvent(new window.Event('change')); + // Hydration shouldn't reset the value to 1 + hydrate(); + + assert.htmlEqual( + target.innerHTML, + '\n2' + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/main.svelte new file mode 100644 index 0000000000..eaf5593198 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input-group/main.svelte @@ -0,0 +1,9 @@ + + +{#each [1, 2, 3] as number} + +{/each} + +{value} diff --git a/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/_config.js b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/_config.js new file mode 100644 index 0000000000..e5bbe5b0fe --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/_config.js @@ -0,0 +1,15 @@ +import { test } from '../../test'; + +export default test({ + skip_mode: ['client'], + + test({ assert, target, hydrate }) { + const input = /** @type {HTMLInputElement} */ (target.querySelector('input')); + input.value = 'foo'; + input.dispatchEvent(new window.Event('input')); + // Hydration shouldn't reset the value to empty + hydrate(); + + assert.htmlEqual(target.innerHTML, '\nfoo'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/main.svelte new file mode 100644 index 0000000000..856667732d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydrate-modified-input/main.svelte @@ -0,0 +1,6 @@ + + + +{value} diff --git a/playgrounds/demo/src/entry-client.ts b/playgrounds/demo/src/entry-client.ts index 8ec831e425..6675efdd19 100644 --- a/playgrounds/demo/src/entry-client.ts +++ b/playgrounds/demo/src/entry-client.ts @@ -4,8 +4,11 @@ import App from './App.svelte'; const root = document.getElementById('root')!; const render = root.firstChild?.nextSibling ? hydrate : mount; -const component = render(App, { - target: document.getElementById('root')! -}); -// @ts-ignore -window.unmount = () => unmount(component); +setTimeout(() => { + const component = render(App, { + target: document.getElementById('root')! + }); + // @ts-ignore + window.unmount = () => unmount(component); +}, 2000) +