From 58008479fc8478edebdc6e343738b856bb50d8a9 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 21 Feb 2024 13:03:18 +0000 Subject: [PATCH 1/7] fix: improve $inspect handling of derived objects (#10584) * fix: improve $inspect handling of derived objects * Update packages/svelte/src/internal/client/runtime.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/khaki-cooks-develop.md | 5 ++ .../svelte/src/internal/client/runtime.js | 12 ++++- .../samples/inspect-derived-2/_config.js | 54 +++++++++++++++++++ .../samples/inspect-derived-2/main.svelte | 21 ++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 .changeset/khaki-cooks-develop.md create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/inspect-derived-2/main.svelte diff --git a/.changeset/khaki-cooks-develop.md b/.changeset/khaki-cooks-develop.md new file mode 100644 index 0000000000..530d581132 --- /dev/null +++ b/.changeset/khaki-cooks-develop.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improve $inspect handling of derived objects diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 8c93d1c158..e348d25f82 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -43,6 +43,7 @@ let is_micro_task_queued = false; let is_flushing_effect = false; // Used for $inspect export let is_batching_effect = false; +let is_inspecting_signal = false; // Handle effect queues @@ -130,8 +131,15 @@ export function batch_inspect(target, prop, receiver) { return Reflect.apply(value, this, arguments); } finally { is_batching_effect = previously_batching_effect; - if (last_inspected_signal !== null) { - for (const fn of last_inspected_signal.inspect) fn(); + if (last_inspected_signal !== null && !is_inspecting_signal) { + is_inspecting_signal = true; + try { + for (const fn of last_inspected_signal.inspect) { + fn(); + } + } finally { + is_inspecting_signal = false; + } last_inspected_signal = null; } } diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js new file mode 100644 index 0000000000..0e6cf85884 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/_config.js @@ -0,0 +1,54 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +/** + * @type {any[]} + */ +let log; +/** + * @type {typeof console.log}} + */ +let original_log; + +export default test({ + compileOptions: { + dev: true + }, + before_test() { + log = []; + original_log = console.log; + console.log = (...v) => { + log.push(...v); + }; + }, + after_test() { + console.log = original_log; + }, + async test({ assert, target }) { + const button = target.querySelector('button'); + + flushSync(() => { + button?.click(); + }); + + assert.htmlEqual(target.innerHTML, `\n1`); + assert.deepEqual(log, [ + 'init', + { + data: { + derived: 0, + list: [] + }, + derived: [] + }, + 'update', + { + data: { + derived: 0, + list: [1] + }, + derived: [1] + } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/main.svelte new file mode 100644 index 0000000000..49dec62f28 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/inspect-derived-2/main.svelte @@ -0,0 +1,21 @@ + + + + + +{state.data.list} From 66b624491eab0d0528060f5340a3b8108a935066 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:30:29 +0100 Subject: [PATCH 2/7] fix: allow boolean `contenteditable` attribute (#10590) fixes #10559 --- .changeset/spotty-turkeys-sparkle.md | 5 +++++ packages/svelte/src/compiler/errors.js | 4 ---- packages/svelte/src/compiler/phases/2-analyze/validation.js | 2 +- .../validator/samples/contenteditable-dynamic/errors.json | 4 ++-- .../validator/samples/contenteditable-dynamic/input.svelte | 5 +++++ 5 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 .changeset/spotty-turkeys-sparkle.md diff --git a/.changeset/spotty-turkeys-sparkle.md b/.changeset/spotty-turkeys-sparkle.md new file mode 100644 index 0000000000..6464bd1e29 --- /dev/null +++ b/.changeset/spotty-turkeys-sparkle.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: allow boolean `contenteditable` attribute diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index f6f70ec3e3..f2207c9336 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -371,10 +371,6 @@ const errors = { // message: // "'contenteditable' attribute is required for textContent, innerHTML and innerText two-way bindings" // }, - // dynamic_contenteditable_attribute: { - // code: 'dynamic-contenteditable-attribute', - // message: "'contenteditable' attribute cannot be dynamic if element uses two-way binding" - // }, // textarea_duplicate_value: { // code: 'textarea-duplicate-value', // message: diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index fbf6392fba..27f61bf634 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -475,7 +475,7 @@ const validation = { ); if (!contenteditable) { error(node, 'missing-contenteditable-attribute'); - } else if (!is_text_attribute(contenteditable)) { + } else if (!is_text_attribute(contenteditable) && contenteditable.value !== true) { error(contenteditable, 'dynamic-contenteditable-attribute'); } } diff --git a/packages/svelte/tests/validator/samples/contenteditable-dynamic/errors.json b/packages/svelte/tests/validator/samples/contenteditable-dynamic/errors.json index 6272bc6d21..27cc3e98c7 100644 --- a/packages/svelte/tests/validator/samples/contenteditable-dynamic/errors.json +++ b/packages/svelte/tests/validator/samples/contenteditable-dynamic/errors.json @@ -3,11 +3,11 @@ "code": "dynamic-contenteditable-attribute", "message": "'contenteditable' attribute cannot be dynamic if element uses two-way binding", "start": { - "line": 6, + "line": 11, "column": 8 }, "end": { - "line": 6, + "line": 11, "column": 32 } } diff --git a/packages/svelte/tests/validator/samples/contenteditable-dynamic/input.svelte b/packages/svelte/tests/validator/samples/contenteditable-dynamic/input.svelte index 8dfa91a354..48b4eadbcc 100644 --- a/packages/svelte/tests/validator/samples/contenteditable-dynamic/input.svelte +++ b/packages/svelte/tests/validator/samples/contenteditable-dynamic/input.svelte @@ -3,4 +3,9 @@ let toggle = false; + + + + + From a2fbef20500de2b0bd0027da6b510e6fdfc6b8a3 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 21 Feb 2024 15:13:45 +0000 Subject: [PATCH 3/7] fix: improve import event handler support (#10592) * fix: improve import event handler support * simplify --- .changeset/yellow-taxis-double.md | 5 +++++ .../3-transform/client/visitors/template.js | 1 + .../samples/event-attribute-import/_config.js | 20 +++++++++++++++++++ .../samples/event-attribute-import/event.js | 14 +++++++++++++ .../event-attribute-import/main.svelte | 6 ++++++ 5 files changed, 46 insertions(+) create mode 100644 .changeset/yellow-taxis-double.md create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte diff --git a/.changeset/yellow-taxis-double.md b/.changeset/yellow-taxis-double.md new file mode 100644 index 0000000000..b8c37c5a89 --- /dev/null +++ b/.changeset/yellow-taxis-double.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improve import event handler support 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 d33be2fdce..fd48fda1b7 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 @@ -1314,6 +1314,7 @@ function serialize_event_handler(node, { state, visit }) { binding !== null && (binding.kind === 'state' || binding.kind === 'frozen_state' || + binding.declaration_kind === 'import' || binding.kind === 'legacy_reactive' || binding.kind === 'derived' || binding.kind === 'prop' || diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js new file mode 100644 index 0000000000..66bda3def6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js @@ -0,0 +1,20 @@ +import { test } from '../../test'; +import { log, handler, log_a } from './event.js'; + +export default test({ + before_test() { + log.length = 0; + handler.value = log_a; + }, + + async test({ assert, target }) { + const [b1, b2] = target.querySelectorAll('button'); + + b1?.click(); + assert.deepEqual(log, ['a']); + + b2?.click(); + b1?.click(); + assert.deepEqual(log, ['a', 'b']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js new file mode 100644 index 0000000000..978f672d30 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js @@ -0,0 +1,14 @@ +/** @type {any[]} */ +export const log = []; + +export const log_a = () => { + log.push('a'); +}; + +export const log_b = () => { + log.push('b'); +}; + +export const handler = { + value: log_a +}; diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte new file mode 100644 index 0000000000..1b743653a7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte @@ -0,0 +1,6 @@ + + + + From ec52c75cc4e4d256ea1cab1d6204cdeb3e10d23c Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 21 Feb 2024 15:24:36 +0000 Subject: [PATCH 4/7] fix: permit whitespace within template scripts (#10591) --- .changeset/rotten-experts-relax.md | 5 +++++ .../phases/3-transform/client/visitors/template.js | 2 +- .../runtime-legacy/samples/head-script/_config.js | 8 ++++++++ .../runtime-legacy/samples/head-script/main.svelte | 10 ++++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 .changeset/rotten-experts-relax.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/head-script/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/head-script/main.svelte diff --git a/.changeset/rotten-experts-relax.md b/.changeset/rotten-experts-relax.md new file mode 100644 index 0000000000..e5a7e3bbe4 --- /dev/null +++ b/.changeset/rotten-experts-relax.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: permit whitespace within template scripts 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 fd48fda1b7..1b95031bb6 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 @@ -2081,7 +2081,7 @@ export const template_visitors = { node.fragment.nodes, context.path, child_metadata.namespace, - state.preserve_whitespace, + node.name === 'script' || state.preserve_whitespace, state.options.preserveComments ); diff --git a/packages/svelte/tests/runtime-legacy/samples/head-script/_config.js b/packages/svelte/tests/runtime-legacy/samples/head-script/_config.js new file mode 100644 index 0000000000..31acab66a3 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/head-script/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, component, window }) { + document.dispatchEvent(new Event('DOMContentLoaded')); + assert.equal(window.document.querySelector('button')?.textContent, 'Hello world'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/head-script/main.svelte b/packages/svelte/tests/runtime-legacy/samples/head-script/main.svelte new file mode 100644 index 0000000000..8284f84a59 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/head-script/main.svelte @@ -0,0 +1,10 @@ + + + +'); const { button } = component; diff --git a/packages/svelte/tests/runtime-legacy/samples/binding-this-with-context/_config.js b/packages/svelte/tests/runtime-legacy/samples/binding-this-with-context/_config.js index 6a4abf3edc..1688d15260 100644 --- a/packages/svelte/tests/runtime-legacy/samples/binding-this-with-context/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/binding-this-with-context/_config.js @@ -46,10 +46,8 @@ export default test({ component.items = ['foo', 'baz']; assert.equal(component.divs.length, 3, 'the divs array is still 3 long'); assert.equal(component.divs[2], null, 'the last div is unregistered'); - // Different from Svelte 3 - assert.equal(component.ps[1], null, 'the second p is unregistered'); - // Different from Svelte 3 - assert.equal(component.spans['-baz2'], null, 'the baz span is unregistered'); + assert.equal(component.ps[2], null, 'the last p is unregistered'); + assert.equal(component.spans['-bar1'], null, 'the bar span is unregistered'); assert.equal(component.hrs.bar, null, 'the bar hr is unregistered'); divs = target.querySelectorAll('div'); @@ -58,22 +56,17 @@ export default test({ assert.equal(e, divs[i], `div ${i} is still correct`); }); - // TODO: unsure if need these two tests to pass, as the logic between Svelte 3 - // and Svelte 5 is different for these cases. - - // elems = target.querySelectorAll('span'); - // component.items.forEach((e, i) => { - // assert.equal( - // component.spans[`-${e}${i}`], - // elems[i], - // `span -${e}${i} is still correct`, - // ); - // }); + spans = target.querySelectorAll('span'); + // @ts-ignore + component.items.forEach((e, i) => { + assert.equal(component.spans[`-${e}${i}`], spans[i], `span -${e}${i} is still correct`); + }); - // elems = target.querySelectorAll('p'); - // component.ps.forEach((e, i) => { - // assert.equal(e, elems[i], `p ${i} is still correct`); - // }); + ps = target.querySelectorAll('p'); + // @ts-ignore + component.ps.forEach((e, i) => { + assert.equal(e, ps[i], `p ${i} is still correct`); + }); hrs = target.querySelectorAll('hr'); // @ts-ignore diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-if-block-outro-timeout/_config.js b/packages/svelte/tests/runtime-legacy/samples/transition-js-if-block-outro-timeout/_config.js index 6c4987cbf6..a110fbd042 100644 --- a/packages/svelte/tests/runtime-legacy/samples/transition-js-if-block-outro-timeout/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/transition-js-if-block-outro-timeout/_config.js @@ -1,3 +1,4 @@ +import { flushSync } from 'svelte'; import { test } from '../../test'; export default test({ @@ -15,7 +16,8 @@ export default test({ assert.equal(window.getComputedStyle(div).opacity, '0'); raf.tick(600); - assert.equal(component.div, undefined); assert.equal(target.querySelector('div'), undefined); + flushSync(); + assert.equal(component.div, undefined); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/_config.js new file mode 100644 index 0000000000..6d428f6306 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/_config.js @@ -0,0 +1,43 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +/** @param {number | null} selected */ +function get_html(selected) { + return ` + + + + +
+ + ${selected !== null ? `
${selected}
` : ''} + +
+ +

${selected ?? '...'}

+ `; +} + +export default test({ + html: get_html(null), + + async test({ assert, target }) { + const [btn1, btn2, btn3] = target.querySelectorAll('button'); + + await btn1?.click(); + await tick(); + assert.htmlEqual(target.innerHTML, get_html(1)); + + await btn2?.click(); + await tick(); + assert.htmlEqual(target.innerHTML, get_html(2)); + + await btn1?.click(); + await tick(); + assert.htmlEqual(target.innerHTML, get_html(1)); + + await btn3?.click(); + await tick(); + assert.htmlEqual(target.innerHTML, get_html(3)); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/main.svelte new file mode 100644 index 0000000000..be400d1d9c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-this-no-state/main.svelte @@ -0,0 +1,30 @@ + + +{#each [1, 2, 3] as n, i} + +{/each} + +
+ +{#each [1, 2, 3] as n, i} + {#if selected === i} +
{n}
+ {/if} +{/each} + +
+ +

{current ?? '...'}

\ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js index 0f261e1c81..78ae1ffc52 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/client/index.svelte.js @@ -11,7 +11,7 @@ export default function Bind_this($$anchor, $$props) { var fragment = $.comment($$anchor); var node = $.child_frag(fragment); - $.bind_this(Foo(node, {}), ($$value) => foo = $$value, foo); + $.bind_this(Foo(node, {}), ($$value) => foo = $$value, () => foo); $.close_frag($$anchor, fragment); $.pop(); } \ No newline at end of file From 6afaf75a378d8514c3d5c60716c600bb402d3de5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 22 Feb 2024 07:58:33 -0500 Subject: [PATCH 7/7] fix: synchronise element bindings (#10512) fixes #10511 We used the animation frame dance previously where the `$effect` timing was slightly different and did not wait on its children before rerunning. With that changed now everything false into place nicely. --------- Co-authored-by: Rich Harris Co-authored-by: Simon Holthausen --- .changeset/quiet-berries-end.md | 5 ++++ packages/svelte/src/internal/client/render.js | 8 +++---- .../_config.js | 13 +++++++++++ .../binding-width-height-this-timing/log.js | 2 ++ .../main.svelte | 23 +++++++++++++++++++ 5 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 .changeset/quiet-berries-end.md create mode 100644 packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/_config.js create mode 100644 packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/log.js create mode 100644 packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/main.svelte diff --git a/.changeset/quiet-berries-end.md b/.changeset/quiet-berries-end.md new file mode 100644 index 0000000000..155b7915f8 --- /dev/null +++ b/.changeset/quiet-berries-end.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: tweak initial `bind:clientWidth/clientHeight/offsetWidth/offsetHeight` update timing diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index b3a048183b..97cb748bae 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -970,11 +970,11 @@ export function bind_resize_observer(dom, type, update) { * @param {(size: number) => void} update */ export function bind_element_size(dom, type, update) { - // We need to wait a few ticks to be sure that the element has been inserted and rendered - // The alternative would be a mutation observer on the document but that's way to expensive - requestAnimationFrame(() => requestAnimationFrame(() => update(dom[type]))); const unsub = resize_observer_border_box.observe(dom, () => update(dom[type])); - render_effect(() => unsub); + effect(() => { + untrack(() => update(dom[type])); + return unsub; + }); } /** diff --git a/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/_config.js b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/_config.js new file mode 100644 index 0000000000..5df9ca4ba2 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/_config.js @@ -0,0 +1,13 @@ +import { test } from '../../assert'; +import { log } from './log.js'; + +export default test({ + async test({ assert }) { + await new Promise((fulfil) => setTimeout(fulfil, 0)); + + assert.deepEqual(log, [ + [false, 0, 0], + [true, 100, 100] + ]); + } +}); diff --git a/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/log.js b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/log.js new file mode 100644 index 0000000000..d3df521f4d --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/log.js @@ -0,0 +1,2 @@ +/** @type {any[]} */ +export const log = []; diff --git a/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/main.svelte b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/main.svelte new file mode 100644 index 0000000000..7437bbc229 --- /dev/null +++ b/packages/svelte/tests/runtime-browser/samples/binding-width-height-this-timing/main.svelte @@ -0,0 +1,23 @@ + + +
+ +