From eca1b7fd06dc176ff9be878024e90a9bc12dea7a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 20 Jun 2024 12:28:48 +0100 Subject: [PATCH 1/4] fix: ensure state update expressions are serialised correctly (#12109) Fixes #12103 --- .changeset/gorgeous-boxes-design.md | 5 +++++ .../compiler/phases/3-transform/client/utils.js | 16 ++++++++++++++-- .../phases/3-transform/client/visitors/global.js | 9 +++++++-- .../3-transform/client/visitors/template.js | 11 +++++++---- .../samples/state-update/_config.js | 7 +++++++ .../samples/state-update/main.svelte | 9 +++++++++ 6 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 .changeset/gorgeous-boxes-design.md create mode 100644 packages/svelte/tests/runtime-runes/samples/state-update/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-update/main.svelte diff --git a/.changeset/gorgeous-boxes-design.md b/.changeset/gorgeous-boxes-design.md new file mode 100644 index 0000000000..5496f6f1fd --- /dev/null +++ b/.changeset/gorgeous-boxes-design.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: ensure state update expressions are serialised correctly diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index c6b10bc2d0..89a1a5000e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -119,10 +119,11 @@ export function serialize_get_binding(node, state) { * @param {import('estree').AssignmentExpression} node * @param {import('zimmerframe').Context} context * @param {() => any} fallback + * @param {boolean} prefix * @param {{skip_proxy_and_freeze?: boolean}} [options] * @returns {import('estree').Expression} */ -export function serialize_set_binding(node, context, fallback, options) { +export function serialize_set_binding(node, context, fallback, prefix, options) { const { state, visit } = context; const assignee = node.left; @@ -146,7 +147,9 @@ export function serialize_set_binding(node, context, fallback, options) { const value = path.expression?.(b.id(tmp_id)); const assignment = b.assignment('=', path.node, value); original_assignments.push(assignment); - assignments.push(serialize_set_binding(assignment, context, () => assignment, options)); + assignments.push( + serialize_set_binding(assignment, context, () => assignment, prefix, options) + ); } if (assignments.every((assignment, i) => assignment === original_assignments[i])) { @@ -411,6 +414,15 @@ export function serialize_set_binding(node, context, fallback, options) { ) ); } + } else if ( + node.right.type === 'Literal' && + (node.operator === '+=' || node.operator === '-=') + ) { + return b.update( + node.operator === '+=' ? '++' : '--', + /** @type {import('estree').Expression} */ (visit(node.left)), + prefix + ); } else { return b.assignment( node.operator, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index e229b5edf7..2834e9ff06 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -32,7 +32,7 @@ export const global_visitors = { next(); }, AssignmentExpression(node, context) { - return serialize_set_binding(node, context, context.next); + return serialize_set_binding(node, context, context.next, false); }, UpdateExpression(node, context) { const { state, next, visit } = context; @@ -98,7 +98,12 @@ export const global_visitors = { /** @type {import('estree').Pattern} */ (argument), b.literal(1) ); - const serialized_assignment = serialize_set_binding(assignment, context, () => assignment); + const serialized_assignment = serialize_set_binding( + assignment, + context, + () => assignment, + node.prefix + ); const value = /** @type {import('estree').Expression} */ (visit(argument)); if (serialized_assignment === assignment) { // No change to output -> nothing to transform -> we can keep the original update expression 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 d8ae6749f4..823b92415b 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 @@ -798,7 +798,9 @@ function serialize_inline_component(node, component_name, context) { const assignment = b.assignment('=', attribute.expression, b.id('$$value')); push_prop( b.set(attribute.name, [ - b.stmt(serialize_set_binding(assignment, context, () => context.visit(assignment))) + b.stmt( + serialize_set_binding(assignment, context, () => context.visit(assignment), false) + ) ]) ); } @@ -1026,7 +1028,7 @@ function serialize_bind_this(bind_this, context, node) { const bind_this_id = /** @type {import('estree').Expression} */ (context.visit(bind_this)); const ids = Array.from(each_ids.values()).map((id) => b.id('$$value_' + id[0])); const assignment = b.assignment('=', bind_this, b.id('$$value')); - const update = serialize_set_binding(assignment, context, () => context.visit(assignment)); + const update = serialize_set_binding(assignment, context, () => context.visit(assignment), false); for (const [binding, [, , expression]] of each_ids) { // reset expressions to what they were before @@ -2400,7 +2402,7 @@ export const template_visitors = { if (assignment.left.type !== 'Identifier' && assignment.left.type !== 'MemberExpression') { // serialize_set_binding turns other patterns into IIFEs and separates the assignments // into separate expressions, at which point this is called again with an identifier or member expression - return serialize_set_binding(assignment, context, () => assignment); + return serialize_set_binding(assignment, context, () => assignment, false); } const left = object(assignment.left); const value = get_assignment_value(assignment, context); @@ -2438,7 +2440,7 @@ export const template_visitors = { : b.id(node.index); const item = each_node_meta.item; const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(item.name)); - binding.expression = (id) => { + binding.expression = (/** @type {import("estree").Identifier} */ id) => { const item_with_loc = with_loc(item, id); return b.call('$.unwrap', item_with_loc); }; @@ -2762,6 +2764,7 @@ export const template_visitors = { assignment, context, () => /** @type {import('estree').Expression} */ (visit(assignment)), + false, { skip_proxy_and_freeze: true } diff --git a/packages/svelte/tests/runtime-runes/samples/state-update/_config.js b/packages/svelte/tests/runtime-runes/samples/state-update/_config.js new file mode 100644 index 0000000000..9d183557f9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-update/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, [1, 1, 1, 1]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-update/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-update/main.svelte new file mode 100644 index 0000000000..011a3f6931 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-update/main.svelte @@ -0,0 +1,9 @@ + From e9e7d8b4683a064642113220bac0a545567680df Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 20 Jun 2024 12:34:50 +0100 Subject: [PATCH 2/4] chore: add more benchmarks (#12108) * chore: add more benchmarks * address feedback --- benchmarking/benchmarks.js | 70 +++- .../benchmarks/kairo/kairo_avoidable.js | 35 +- benchmarking/benchmarks/kairo/kairo_broad.js | 37 +- benchmarking/benchmarks/kairo/kairo_deep.js | 49 ++- .../benchmarks/kairo/kairo_diamond.js | 57 ++- benchmarking/benchmarks/kairo/kairo_mux.js | 47 ++- .../benchmarks/kairo/kairo_repeated.js | 51 ++- .../benchmarks/kairo/kairo_triangle.js | 67 +++- .../benchmarks/kairo/kairo_unstable.js | 55 ++- benchmarking/benchmarks/mol_bench.js | 35 +- benchmarking/benchmarks/sbench.js | 339 ++++++++++++++++++ 11 files changed, 747 insertions(+), 95 deletions(-) create mode 100644 benchmarking/benchmarks/sbench.js diff --git a/benchmarking/benchmarks.js b/benchmarking/benchmarks.js index 283112dbe3..795ac3e09e 100644 --- a/benchmarking/benchmarks.js +++ b/benchmarking/benchmarks.js @@ -1,24 +1,58 @@ -import { kairo_avoidable } from './benchmarks/kairo/kairo_avoidable.js'; -import { kairo_broad } from './benchmarks/kairo/kairo_broad.js'; -import { kairo_deep } from './benchmarks/kairo/kairo_deep.js'; -import { kairo_diamond } from './benchmarks/kairo/kairo_diamond.js'; -import { kairo_mux } from './benchmarks/kairo/kairo_mux.js'; -import { kairo_repeated } from './benchmarks/kairo/kairo_repeated.js'; -import { kairo_triangle } from './benchmarks/kairo/kairo_triangle.js'; -import { kairo_unstable } from './benchmarks/kairo/kairo_unstable.js'; -import { mol_bench } from './benchmarks/mol_bench.js'; +import { + kairo_avoidable_owned, + kairo_avoidable_unowned +} from './benchmarks/kairo/kairo_avoidable.js'; +import { kairo_broad_owned, kairo_broad_unowned } from './benchmarks/kairo/kairo_broad.js'; +import { kairo_deep_owned, kairo_deep_unowned } from './benchmarks/kairo/kairo_deep.js'; +import { kairo_diamond_owned, kairo_diamond_unowned } from './benchmarks/kairo/kairo_diamond.js'; +import { kairo_mux_unowned, kairo_mux_owned } from './benchmarks/kairo/kairo_mux.js'; +import { kairo_repeated_unowned, kairo_repeated_owned } from './benchmarks/kairo/kairo_repeated.js'; +import { kairo_triangle_owned, kairo_triangle_unowned } from './benchmarks/kairo/kairo_triangle.js'; +import { kairo_unstable_owned, kairo_unstable_unowned } from './benchmarks/kairo/kairo_unstable.js'; +import { mol_bench_owned, mol_bench_unowned } from './benchmarks/mol_bench.js'; +import { + sbench_create_0to1, + sbench_create_1000to1, + sbench_create_1to1, + sbench_create_1to1000, + sbench_create_1to2, + sbench_create_1to4, + sbench_create_1to8, + sbench_create_2to1, + sbench_create_4to1, + sbench_create_signals +} from './benchmarks/sbench.js'; // This benchmark has been adapted from the js-reactivity-benchmark (https://github.com/milomg/js-reactivity-benchmark) // Not all tests are the same, and many parts have been tweaked to capture different data. export const benchmarks = [ - kairo_avoidable, - kairo_broad, - kairo_deep, - kairo_diamond, - kairo_triangle, - kairo_mux, - kairo_repeated, - kairo_unstable, - mol_bench + sbench_create_signals, + sbench_create_0to1, + sbench_create_1to1, + sbench_create_2to1, + sbench_create_4to1, + sbench_create_1000to1, + sbench_create_1to2, + sbench_create_1to4, + sbench_create_1to8, + sbench_create_1to1000, + kairo_avoidable_owned, + kairo_avoidable_unowned, + kairo_broad_owned, + kairo_broad_unowned, + kairo_deep_owned, + kairo_deep_unowned, + kairo_diamond_owned, + kairo_diamond_unowned, + kairo_triangle_owned, + kairo_triangle_unowned, + kairo_mux_owned, + kairo_mux_unowned, + kairo_repeated_owned, + kairo_repeated_unowned, + kairo_unstable_owned, + kairo_unstable_unowned, + mol_bench_owned, + mol_bench_unowned ]; diff --git a/benchmarking/benchmarks/kairo/kairo_avoidable.js b/benchmarking/benchmarks/kairo/kairo_avoidable.js index 636e96ccce..108f79458c 100644 --- a/benchmarking/benchmarks/kairo/kairo_avoidable.js +++ b/benchmarking/benchmarks/kairo/kairo_avoidable.js @@ -34,7 +34,7 @@ function setup() { }; } -export async function kairo_avoidable() { +export async function kairo_avoidable_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -53,7 +53,38 @@ export async function kairo_avoidable() { destroy(); return { - benchmark: 'kairo_avoidable', + benchmark: 'kairo_avoidable_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_avoidable_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_avoidable_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_broad.js b/benchmarking/benchmarks/kairo/kairo_broad.js index 682154f0e3..06ce3cd7b5 100644 --- a/benchmarking/benchmarks/kairo/kairo_broad.js +++ b/benchmarking/benchmarks/kairo/kairo_broad.js @@ -28,7 +28,7 @@ function setup() { $.flush_sync(() => { $.set(head, 1); }); - counter = 0 + counter = 0; for (let i = 0; i < 50; i++) { $.flush_sync(() => { $.set(head, i); @@ -40,7 +40,7 @@ function setup() { }; } -export async function kairo_broad() { +export async function kairo_broad_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -59,7 +59,38 @@ export async function kairo_broad() { destroy(); return { - benchmark: 'kairo_broad', + benchmark: 'kairo_broad_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_broad_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_broad_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_deep.js b/benchmarking/benchmarks/kairo/kairo_deep.js index af985c3e43..ac4ac5641b 100644 --- a/benchmarking/benchmarks/kairo/kairo_deep.js +++ b/benchmarking/benchmarks/kairo/kairo_deep.js @@ -8,12 +8,12 @@ function setup() { let head = $.source(0); let current = head; for (let i = 0; i < len; i++) { - let c = current; - current = $.derived(() => { - return $.get(c) + 1; - }); - } - let counter = 0; + let c = current; + current = $.derived(() => { + return $.get(c) + 1; + }); + } + let counter = 0; const destroy = $.effect_root(() => { $.render_effect(() => { @@ -28,7 +28,7 @@ function setup() { $.flush_sync(() => { $.set(head, 1); }); - counter = 0 + counter = 0; for (let i = 0; i < iter; i++) { $.flush_sync(() => { $.set(head, i); @@ -40,7 +40,7 @@ function setup() { }; } -export async function kairo_deep() { +export async function kairo_deep_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -59,7 +59,38 @@ export async function kairo_deep() { destroy(); return { - benchmark: 'kairo_deep', + benchmark: 'kairo_deep_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_deep_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_deep_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_diamond.js b/benchmarking/benchmarks/kairo/kairo_diamond.js index 879727c99e..09d65cd89a 100644 --- a/benchmarking/benchmarks/kairo/kairo_diamond.js +++ b/benchmarking/benchmarks/kairo/kairo_diamond.js @@ -5,17 +5,17 @@ let width = 5; function setup() { let head = $.source(0); - let current = []; - for (let i = 0; i < width; i++) { - current.push( - $.derived(() => { - return $.get(head) + 1; - }) - ); - } - let sum = $.derived(() => { - return current.map((x) => $.get(x)).reduce((a, b) => a + b, 0); - }); + let current = []; + for (let i = 0; i < width; i++) { + current.push( + $.derived(() => { + return $.get(head) + 1; + }) + ); + } + let sum = $.derived(() => { + return current.map((x) => $.get(x)).reduce((a, b) => a + b, 0); + }); let counter = 0; const destroy = $.effect_root(() => { @@ -44,7 +44,7 @@ function setup() { }; } -export async function kairo_diamond() { +export async function kairo_diamond_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -63,7 +63,38 @@ export async function kairo_diamond() { destroy(); return { - benchmark: 'kairo_diamond', + benchmark: 'kairo_diamond_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_diamond_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_diamond_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_mux.js b/benchmarking/benchmarks/kairo/kairo_mux.js index d2867f499b..903b8d3758 100644 --- a/benchmarking/benchmarks/kairo/kairo_mux.js +++ b/benchmarking/benchmarks/kairo/kairo_mux.js @@ -3,12 +3,12 @@ import * as $ from '../../../packages/svelte/src/internal/client/index.js'; function setup() { let heads = new Array(100).fill(null).map((_) => $.source(0)); - const mux = $.derived(() => { - return Object.fromEntries(heads.map((h) => $.get(h)).entries()); - }); - const splited = heads - .map((_, index) => $.derived(() => $.get(mux)[index])) - .map((x) => $.derived(() => $.get(x) + 1)); + const mux = $.derived(() => { + return Object.fromEntries(heads.map((h) => $.get(h)).entries()); + }); + const splited = heads + .map((_, index) => $.derived(() => $.get(mux)[index])) + .map((x) => $.derived(() => $.get(x) + 1)); const destroy = $.effect_root(() => { splited.forEach((x) => { @@ -37,7 +37,7 @@ function setup() { }; } -export async function kairo_mux() { +export async function kairo_mux_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -56,7 +56,38 @@ export async function kairo_mux() { destroy(); return { - benchmark: 'kairo_mux', + benchmark: 'kairo_mux_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_mux_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_mux_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_repeated.js b/benchmarking/benchmarks/kairo/kairo_repeated.js index fd22c1e563..ef9c284ad0 100644 --- a/benchmarking/benchmarks/kairo/kairo_repeated.js +++ b/benchmarking/benchmarks/kairo/kairo_repeated.js @@ -4,14 +4,14 @@ import * as $ from '../../../packages/svelte/src/internal/client/index.js'; let size = 30; function setup() { - let head = $.source(0); - let current = $.derived(() => { - let result = 0; - for (let i = 0; i < size; i++) { - result += $.get(head); - } - return result; - }); + let head = $.source(0); + let current = $.derived(() => { + let result = 0; + for (let i = 0; i < size; i++) { + result += $.get(head); + } + return result; + }); let counter = 0; @@ -41,7 +41,7 @@ function setup() { }; } -export async function kairo_repeated() { +export async function kairo_repeated_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -60,7 +60,38 @@ export async function kairo_repeated() { destroy(); return { - benchmark: 'kairo_repeated', + benchmark: 'kairo_repeated_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_repeated_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_repeated_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_triangle.js b/benchmarking/benchmarks/kairo/kairo_triangle.js index 4e8afe1b82..0696efb39e 100644 --- a/benchmarking/benchmarks/kairo/kairo_triangle.js +++ b/benchmarking/benchmarks/kairo/kairo_triangle.js @@ -4,26 +4,26 @@ import * as $ from '../../../packages/svelte/src/internal/client/index.js'; let width = 10; function count(number) { - return new Array(number) - .fill(0) - .map((_, i) => i + 1) - .reduce((x, y) => x + y, 0); + return new Array(number) + .fill(0) + .map((_, i) => i + 1) + .reduce((x, y) => x + y, 0); } function setup() { let head = $.source(0); - let current = head; - let list = []; - for (let i = 0; i < width; i++) { - let c = current; - list.push(current); - current = $.derived(() => { - return $.get(c) + 1; - }); - } - let sum = $.derived(() => { - return list.map((x) => $.get(x)).reduce((a, b) => a + b, 0); - }); + let current = head; + let list = []; + for (let i = 0; i < width; i++) { + let c = current; + list.push(current); + current = $.derived(() => { + return $.get(c) + 1; + }); + } + let sum = $.derived(() => { + return list.map((x) => $.get(x)).reduce((a, b) => a + b, 0); + }); let counter = 0; @@ -54,7 +54,7 @@ function setup() { }; } -export async function kairo_triangle() { +export async function kairo_triangle_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -73,7 +73,38 @@ export async function kairo_triangle() { destroy(); return { - benchmark: 'kairo_triangle', + benchmark: 'kairo_triangle_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_triangle_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_triangle_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/kairo/kairo_unstable.js b/benchmarking/benchmarks/kairo/kairo_unstable.js index 0185d12868..3e7120d6d2 100644 --- a/benchmarking/benchmarks/kairo/kairo_unstable.js +++ b/benchmarking/benchmarks/kairo/kairo_unstable.js @@ -2,16 +2,16 @@ import { assert, fastest_test } from '../../utils.js'; import * as $ from '../../../packages/svelte/src/internal/client/index.js'; function setup() { - let head = $.source(0); - const double = $.derived(() => $.get(head) * 2); - const inverse = $.derived(() => -$.get(head)); - let current = $.derived(() => { - let result = 0; - for (let i = 0; i < 20; i++) { - result += $.get(head) % 2 ? $.get(double) : $.get(inverse); - } - return result; - }); + let head = $.source(0); + const double = $.derived(() => $.get(head) * 2); + const inverse = $.derived(() => -$.get(head)); + let current = $.derived(() => { + let result = 0; + for (let i = 0; i < 20; i++) { + result += $.get(head) % 2 ? $.get(double) : $.get(inverse); + } + return result; + }); let counter = 0; @@ -40,7 +40,7 @@ function setup() { }; } -export async function kairo_unstable() { +export async function kairo_unstable_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -59,7 +59,38 @@ export async function kairo_unstable() { destroy(); return { - benchmark: 'kairo_unstable', + benchmark: 'kairo_unstable_unowned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function kairo_unstable_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + run(); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'kairo_unstable_owned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/mol_bench.js b/benchmarking/benchmarks/mol_bench.js index 7fa49d7df4..adb4aceb23 100644 --- a/benchmarking/benchmarks/mol_bench.js +++ b/benchmarking/benchmarks/mol_bench.js @@ -64,7 +64,38 @@ function setup() { }; } -export async function mol_bench() { +export async function mol_bench_owned() { + let run, destroy; + + const destroy_owned = $.effect_root(() => { + // Do 10 loops to warm up JIT + for (let i = 0; i < 10; i++) { + const { run, destroy } = setup(); + run(0); + destroy(); + } + + ({ run, destroy } = setup()); + }); + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 1e4; i++) { + run(i); + } + }); + + // @ts-ignore + destroy(); + destroy_owned(); + + return { + benchmark: 'mol_bench_owned', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function mol_bench_unowned() { // Do 10 loops to warm up JIT for (let i = 0; i < 10; i++) { const { run, destroy } = setup(); @@ -83,7 +114,7 @@ export async function mol_bench() { destroy(); return { - benchmark: 'mol_bench', + benchmark: 'mol_bench_unowned', time: timing.time.toFixed(2), gc_time: timing.gc_time.toFixed(2) }; diff --git a/benchmarking/benchmarks/sbench.js b/benchmarking/benchmarks/sbench.js new file mode 100644 index 0000000000..9bc0cee83c --- /dev/null +++ b/benchmarking/benchmarks/sbench.js @@ -0,0 +1,339 @@ +import { fastest_test } from '../utils.js'; +import * as $ from '../../packages/svelte/src/internal/client/index.js'; + +const COUNT = 1e5; + +/** + * @param {number} n + * @param {any[]} sources + */ +function create_data_signals(n, sources) { + for (let i = 0; i < n; i++) { + sources[i] = $.source(i); + } + return sources; +} + +/** + * @param {number} i + */ +function create_computation_0(i) { + $.derived(() => i); +} + +/** + * @param {any} s1 + */ +function create_computation_1(s1) { + $.derived(() => $.get(s1)); +} +/** + * @param {any} s1 + * @param {any} s2 + */ +function create_computation_2(s1, s2) { + $.derived(() => $.get(s1) + $.get(s2)); +} + +function create_computation_1000(ss, offset) { + $.derived(() => { + let sum = 0; + for (let i = 0; i < 1000; i++) { + sum += $.get(ss[offset + i]); + } + return sum; + }); +} + +/** + * @param {number} n + */ +function create_computations_0to1(n) { + for (let i = 0; i < n; i++) { + create_computation_0(i); + } +} + +/** + * @param {number} n + * @param {any[]} sources + */ +function create_computations_1to1(n, sources) { + for (let i = 0; i < n; i++) { + const source = sources[i]; + create_computation_1(source); + } +} + +/** + * @param {number} n + * @param {any[]} sources + */ +function create_computations_2to1(n, sources) { + for (let i = 0; i < n; i++) { + create_computation_2(sources[i * 2], sources[i * 2 + 1]); + } +} + +function create_computation_4(s1, s2, s3, s4) { + $.derived(() => $.get(s1) + $.get(s2) + $.get(s3) + $.get(s4)); +} + +function create_computations_1000to1(n, sources) { + for (let i = 0; i < n; i++) { + create_computation_1000(sources, i * 1000); + } +} + +function create_computations_1to2(n, sources) { + for (let i = 0; i < n / 2; i++) { + const source = sources[i]; + create_computation_1(source); + create_computation_1(source); + } +} + +function create_computations_1to4(n, sources) { + for (let i = 0; i < n / 4; i++) { + const source = sources[i]; + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + } +} + +function create_computations_1to8(n, sources) { + for (let i = 0; i < n / 8; i++) { + const source = sources[i]; + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + create_computation_1(source); + } +} + +function create_computations_1to1000(n, sources) { + for (let i = 0; i < n / 1000; i++) { + const source = sources[i]; + for (let j = 0; j < 1000; j++) { + create_computation_1(source); + } + } +} + +function create_computations_4to1(n, sources) { + for (let i = 0; i < n; i++) { + create_computation_4( + sources[i * 4], + sources[i * 4 + 1], + sources[i * 4 + 2], + sources[i * 4 + 3] + ); + } +} + +/** + * @param {any} fn + * @param {number} count + * @param {number} scount + */ +function bench(fn, count, scount) { + let sources = create_data_signals(scount, []); + + fn(count, sources); +} + +export async function sbench_create_signals() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_data_signals, COUNT, COUNT); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_data_signals, COUNT, COUNT); + } + }); + + return { + benchmark: 'sbench_create_signals', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_0to1() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_0to1, COUNT, 0); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_0to1, COUNT, 0); + } + }); + + return { + benchmark: 'sbench_create_0to1', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1to1() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1to1, COUNT, COUNT); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1to1, COUNT, COUNT); + } + }); + + return { + benchmark: 'sbench_create_1to1', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_2to1() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_2to1, COUNT / 2, COUNT); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_2to1, COUNT / 2, COUNT); + } + }); + + return { + benchmark: 'sbench_create_2to1', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_4to1() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_4to1, COUNT / 4, COUNT); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_4to1, COUNT / 4, COUNT); + } + }); + + return { + benchmark: 'sbench_create_4to1', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1000to1() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1000to1, COUNT / 1000, COUNT); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1000to1, COUNT / 1000, COUNT); + } + }); + + return { + benchmark: 'sbench_create_1000to1', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1to2() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1to2, COUNT, COUNT / 2); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1to2, COUNT, COUNT / 2); + } + }); + + return { + benchmark: 'sbench_create_1to2', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1to4() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1to4, COUNT, COUNT / 4); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1to4, COUNT, COUNT / 4); + } + }); + + return { + benchmark: 'sbench_create_1to4', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1to8() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1to8, COUNT, COUNT / 8); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1to8, COUNT, COUNT / 8); + } + }); + + return { + benchmark: 'sbench_create_1to8', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} + +export async function sbench_create_1to1000() { + // Do 3 loops to warm up JIT + for (let i = 0; i < 3; i++) { + bench(create_computations_1to1000, COUNT, COUNT / 1000); + } + + const { timing } = await fastest_test(10, () => { + for (let i = 0; i < 100; i++) { + bench(create_computations_1to1000, COUNT, COUNT / 1000); + } + }); + + return { + benchmark: 'sbench_create_1to1000', + time: timing.time.toFixed(2), + gc_time: timing.gc_time.toFixed(2) + }; +} From 6a3e293207934b8f2deb88e2f56a001d9ffcebfb Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 20 Jun 2024 20:26:05 +0100 Subject: [PATCH 3/4] fix: wait a microtask for await blocks to reduce UI churn (#11989) * fix: wait a microtask for await blocks to reduce UI churn * fix: wait a microtask for await blocks to reduce UI churn * fix: wait a microtask for await blocks to reduce UI churn * fix bug * Make then blocks reactive * add test * update test * update test * Update packages/svelte/src/internal/client/dom/blocks/await.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * Add support for catch block * slightly more specific naming * if we use the reserved $$ prefix we dont need to mess around with scope.generate * omit args for then/catch if unnecessary * neaten up some old code * shrink code * simplify test * add failing test * preserve pending blocks * update test * fix comment typo * tidy up --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Rich Harris --- .changeset/gentle-eagles-walk.md | 5 + .../phases/3-transform/client/utils.js | 42 +++++ .../3-transform/client/visitors/template.js | 76 +++++---- packages/svelte/src/compiler/phases/scope.js | 4 +- .../src/internal/client/dom/blocks/await.js | 146 ++++++++++-------- .../_config.js | 1 + .../await-then-no-expression/main.svelte | 2 +- .../_config.js | 38 ++--- .../svelte/tests/runtime-legacy/shared.ts | 4 +- .../await-pending-persistent/_config.js | 24 +++ .../await-pending-persistent/main.svelte | 17 ++ .../samples/await-resolve-2/_config.js | 21 +++ .../samples/await-resolve-2/main.svelte | 21 +++ .../samples/await-resolve/_config.js | 27 ++++ .../samples/await-resolve/main.svelte | 17 ++ 15 files changed, 320 insertions(+), 125 deletions(-) create mode 100644 .changeset/gentle-eagles-walk.md create mode 100644 packages/svelte/tests/runtime-runes/samples/await-pending-persistent/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/await-pending-persistent/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/await-resolve-2/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/await-resolve/main.svelte diff --git a/.changeset/gentle-eagles-walk.md b/.changeset/gentle-eagles-walk.md new file mode 100644 index 0000000000..4ed6c5b3fe --- /dev/null +++ b/.changeset/gentle-eagles-walk.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: wait a microtask for await blocks to reduce UI churn diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 89a1a5000e..884a7addfa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -1,5 +1,6 @@ import * as b from '../../../utils/builders.js'; import { + extract_identifiers, extract_paths, is_expression_async, is_simple_expression, @@ -684,3 +685,44 @@ export function with_loc(target, source) { } return target; } + +/** + * @param {import("estree").Pattern} node + * @param {import("zimmerframe").Context} context + * @returns {{ id: import("estree").Pattern, declarations: null | import("estree").Statement[] }} + */ +export function create_derived_block_argument(node, context) { + if (node.type === 'Identifier') { + return { id: node, declarations: null }; + } + + const pattern = /** @type {import('estree').Pattern} */ (context.visit(node)); + const identifiers = extract_identifiers(node); + + const id = b.id('$$source'); + const value = b.id('$$value'); + + const block = b.block([ + b.var(pattern, b.call('$.get', id)), + b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier)))) + ]); + + const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))]; + + for (const id of identifiers) { + declarations.push( + b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id)))) + ); + } + + return { id, declarations }; +} + +/** + * Svelte legacy mode should use safe equals in most places, runes mode shouldn't + * @param {import('./types.js').ComponentClientTransformState} state + * @param {import('estree').Expression} arg + */ +export function create_derived(state, arg) { + return b.call(state.analysis.runes ? '$.derived' : '$.derived_safe_equal', arg); +} 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 823b92415b..e7d01f33db 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 @@ -21,7 +21,9 @@ import { function_visitor, get_assignment_value, serialize_get_binding, - serialize_set_binding + serialize_set_binding, + create_derived, + create_derived_block_argument } from '../utils.js'; import { AttributeAliases, @@ -646,15 +648,6 @@ function collect_parent_each_blocks(context) { ); } -/** - * Svelte legacy mode should use safe equals in most places, runes mode shouldn't - * @param {import('../types.js').ComponentClientTransformState} state - * @param {import('estree').Expression} arg - */ -function create_derived(state, arg) { - return b.call(state.analysis.runes ? '$.derived' : '$.derived_safe_equal', arg); -} - /** * @param {import('#compiler').Component | import('#compiler').SvelteComponent | import('#compiler').SvelteSelf} node * @param {string} component_name @@ -2594,6 +2587,45 @@ export const template_visitors = { AwaitBlock(node, context) { context.state.template.push(''); + let then_block; + let catch_block; + + if (node.then) { + /** @type {import('estree').Pattern[]} */ + const args = [b.id('$$anchor')]; + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.then)); + + if (node.value) { + const argument = create_derived_block_argument(node.value, context); + + args.push(argument.id); + + if (argument.declarations !== null) { + block.body.unshift(...argument.declarations); + } + } + + then_block = b.arrow(args, block); + } + + if (node.catch) { + /** @type {import('estree').Pattern[]} */ + const args = [b.id('$$anchor')]; + const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.catch)); + + if (node.error) { + const argument = create_derived_block_argument(node.error, context); + + args.push(argument.id); + + if (argument.declarations !== null) { + block.body.unshift(...argument.declarations); + } + } + + catch_block = b.arrow(args, block); + } + context.state.init.push( b.stmt( b.call( @@ -2606,28 +2638,8 @@ export const template_visitors = { /** @type {import('estree').BlockStatement} */ (context.visit(node.pending)) ) : b.literal(null), - node.then - ? b.arrow( - node.value - ? [ - b.id('$$anchor'), - /** @type {import('estree').Pattern} */ (context.visit(node.value)) - ] - : [b.id('$$anchor')], - /** @type {import('estree').BlockStatement} */ (context.visit(node.then)) - ) - : b.literal(null), - node.catch - ? b.arrow( - node.error - ? [ - b.id('$$anchor'), - /** @type {import('estree').Pattern} */ (context.visit(node.error)) - ] - : [b.id('$$anchor')], - /** @type {import('estree').BlockStatement} */ (context.visit(node.catch)) - ) - : b.literal(null) + then_block, + catch_block ) ) ); diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 850ac9a402..604c084ab1 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -613,7 +613,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { scopes.set(node.value, value_scope); context.visit(node.value, { scope: value_scope }); for (const id of extract_identifiers(node.value)) { - then_scope.declare(id, 'normal', 'const'); + then_scope.declare(id, 'derived', 'const'); value_scope.declare(id, 'normal', 'const'); } } @@ -627,7 +627,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { scopes.set(node.error, error_scope); context.visit(node.error, { scope: error_scope }); for (const id of extract_identifiers(node.error)) { - catch_scope.declare(id, 'normal', 'const'); + catch_scope.declare(id, 'derived', 'const'); error_scope.declare(id, 'normal', 'const'); } } diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 8956631a6a..692d3e7458 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -7,111 +7,135 @@ import { set_current_reaction, set_dev_current_component_function } from '../../runtime.js'; -import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; -import { INERT } from '../../constants.js'; +import { block, branch, pause_effect, resume_effect } from '../../reactivity/effects.js'; import { DEV } from 'esm-env'; +import { queue_micro_task } from '../task.js'; +import { hydrating } from '../hydration.js'; +import { set, source } from '../../reactivity/sources.js'; + +const PENDING = 0; +const THEN = 1; +const CATCH = 2; /** * @template V * @param {Comment} anchor * @param {(() => Promise)} get_input * @param {null | ((anchor: Node) => void)} pending_fn - * @param {null | ((anchor: Node, value: V) => void)} then_fn + * @param {null | ((anchor: Node, value: import('#client').Source) => void)} then_fn * @param {null | ((anchor: Node, error: unknown) => void)} catch_fn * @returns {void} */ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { - const component_context = current_component_context; - /** @type {any} */ - let component_function; - if (DEV) { - component_function = component_context?.function ?? null; - } + var component_context = current_component_context; /** @type {any} */ - let input; + var component_function = DEV ? component_context?.function : null; + + /** @type {V | Promise} */ + var input; /** @type {import('#client').Effect | null} */ - let pending_effect; + var pending_effect; /** @type {import('#client').Effect | null} */ - let then_effect; + var then_effect; /** @type {import('#client').Effect | null} */ - let catch_effect; + var catch_effect; + + var input_source = source(/** @type {V} */ (undefined)); + var error_source = source(undefined); + var resolved = false; /** - * @param {(anchor: Comment, value: any) => void} fn - * @param {any} value + * @param {PENDING | THEN | CATCH} state + * @param {boolean} restore */ - function create_effect(fn, value) { - set_current_effect(effect); - set_current_reaction(effect); // TODO do we need both? - set_current_component_context(component_context); - if (DEV) { - set_dev_current_component_function(component_function); + function update(state, restore) { + resolved = true; + + if (restore) { + set_current_effect(effect); + set_current_reaction(effect); // TODO do we need both? + set_current_component_context(component_context); + if (DEV) set_dev_current_component_function(component_function); + } + + if (state === PENDING && pending_fn) { + if (pending_effect) resume_effect(pending_effect); + else pending_effect = branch(() => pending_fn(anchor)); + } + + if (state === THEN && then_fn) { + if (then_effect) resume_effect(then_effect); + else then_effect = branch(() => then_fn(anchor, input_source)); + } + + if (state === CATCH && catch_fn) { + if (catch_effect) resume_effect(catch_effect); + else catch_effect = branch(() => catch_fn(anchor, error_source)); + } + + if (state !== PENDING && pending_effect) { + pause_effect(pending_effect, () => (pending_effect = null)); } - var e = branch(() => fn(anchor, value)); - if (DEV) { - set_dev_current_component_function(null); + + if (state !== THEN && then_effect) { + pause_effect(then_effect, () => (then_effect = null)); + } + + if (state !== CATCH && catch_effect) { + pause_effect(catch_effect, () => (catch_effect = null)); } - set_current_component_context(null); - set_current_reaction(null); - set_current_effect(null); - // without this, the DOM does not update until two ticks after the promise, - // resolves which is unexpected behaviour (and somewhat irksome to test) - flush_sync(); + if (restore) { + if (DEV) set_dev_current_component_function(null); + set_current_component_context(null); + set_current_reaction(null); + set_current_effect(null); - return e; + // without this, the DOM does not update until two ticks after the promise + // resolves, which is unexpected behaviour (and somewhat irksome to test) + flush_sync(); + } } - const effect = block(() => { + var effect = block(() => { if (input === (input = get_input())) return; if (is_promise(input)) { - const promise = /** @type {Promise} */ (input); + var promise = input; - if (pending_fn) { - if (pending_effect && (pending_effect.f & INERT) === 0) { - destroy_effect(pending_effect); - } - - pending_effect = branch(() => pending_fn(anchor)); - } - - if (then_effect) pause_effect(then_effect); - if (catch_effect) pause_effect(catch_effect); + resolved = false; promise.then( (value) => { if (promise !== input) return; - if (pending_effect) pause_effect(pending_effect); - - if (then_fn) { - then_effect = create_effect(then_fn, value); - } + set(input_source, value); + update(THEN, true); }, (error) => { if (promise !== input) return; - if (pending_effect) pause_effect(pending_effect); - - if (catch_fn) { - catch_effect = create_effect(catch_fn, error); - } + set(error_source, error); + update(CATCH, true); } ); - } else { - if (pending_effect) pause_effect(pending_effect); - if (catch_effect) pause_effect(catch_effect); - if (then_fn) { - if (then_effect) { - destroy_effect(then_effect); + if (hydrating) { + if (pending_fn) { + pending_effect = branch(() => pending_fn(anchor)); } - - then_effect = branch(() => then_fn(anchor, input)); + } else { + // Wait a microtask before checking if we should show the pending state as + // the promise might have resolved by the next microtask. + queue_micro_task(() => { + if (!resolved) update(PENDING, true); + }); } + } else { + set(input_source, input); + update(THEN, false); } // Inert effects are proactively detached from the effect tree. Returning a noop diff --git a/packages/svelte/tests/runtime-legacy/samples/await-then-destruct-computed-props/_config.js b/packages/svelte/tests/runtime-legacy/samples/await-then-destruct-computed-props/_config.js index 734fcf6aed..5c873067ee 100644 --- a/packages/svelte/tests/runtime-legacy/samples/await-then-destruct-computed-props/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/await-then-destruct-computed-props/_config.js @@ -22,6 +22,7 @@ export default test({ prop3: { prop7: 'seven' }, prop4: { prop10: 'ten' } })); + await Promise.resolve(); assert.htmlEqual( target.innerHTML, ` diff --git a/packages/svelte/tests/runtime-legacy/samples/await-then-no-expression/main.svelte b/packages/svelte/tests/runtime-legacy/samples/await-then-no-expression/main.svelte index fedc7cd2b7..a48870ae67 100644 --- a/packages/svelte/tests/runtime-legacy/samples/await-then-no-expression/main.svelte +++ b/packages/svelte/tests/runtime-legacy/samples/await-then-no-expression/main.svelte @@ -20,4 +20,4 @@

the promise is pending

{:then}

the promise is resolved

-{/await} \ No newline at end of file +{/await} diff --git a/packages/svelte/tests/runtime-legacy/samples/transition-js-await-block-outros/_config.js b/packages/svelte/tests/runtime-legacy/samples/transition-js-await-block-outros/_config.js index 29537d6ff5..1ce8a85028 100644 --- a/packages/svelte/tests/runtime-legacy/samples/transition-js-await-block-outros/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/transition-js-await-block-outros/_config.js @@ -118,9 +118,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

43

+

44

loading...

-

44

` ); @@ -159,9 +158,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

+

45

loading...

-

45

` ); @@ -169,9 +167,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

+

45

loading...

-

45

` ); @@ -183,10 +180,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

+

45

loading...

-

45

-

loading...

` ); @@ -195,10 +190,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

-

loading...

-

45

-

loading...

+

45

+

loading...

` ); @@ -207,11 +200,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

-

loading...

-

45

-

loading...

-

46

+

46

+

loading...

` ); @@ -219,20 +209,12 @@ export default test({ assert.htmlEqual( target.innerHTML, ` -

44

-

46

+

46

+

loading...

` ); raf.tick((time += 20)); - assert.htmlEqual( - target.innerHTML, - ` -

46

- ` - ); - - raf.tick((time += 70)); assert.htmlEqual( target.innerHTML, ` diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index d7b64302b7..ed32d2f0c1 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -183,7 +183,9 @@ async function run_test_variant( if (str.slice(0, i).includes('logs')) { // eslint-disable-next-line no-console - console.log = (...args) => logs.push(...args); + console.log = (...args) => { + logs.push(...args); + }; } if (str.slice(0, i).includes('hydrate')) { diff --git a/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/_config.js b/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/_config.js new file mode 100644 index 0000000000..d01619e7ec --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/_config.js @@ -0,0 +1,24 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [b1, b2] = target.querySelectorAll('button'); + + b1.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + `

pending

` + ); + + b2.click(); + await Promise.resolve(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + `

pending

` + ); + + assert.deepEqual(logs, ['rendering pending block']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/main.svelte b/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/main.svelte new file mode 100644 index 0000000000..3d1fe303b4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-pending-persistent/main.svelte @@ -0,0 +1,17 @@ + + +{#await promise} + {console.log('rendering pending block')} +

pending

+{:then value} + {console.log('rendering then block')} +

then {value}

+{/await} + + + diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js new file mode 100644 index 0000000000..88f1b1e961 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js @@ -0,0 +1,21 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [b1, b2, b3, b4] = target.querySelectorAll('button'); + b1.click(); + await Promise.resolve(); + b2.click(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + b3.click(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + b4.click(); + await Promise.resolve(); + await Promise.resolve(); + assert.deepEqual(logs, ['pending', 'a', 'b', 'c', 'pending']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/main.svelte new file mode 100644 index 0000000000..4edfa88f33 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/main.svelte @@ -0,0 +1,21 @@ + + +{#await current_promise} + {console.log('pending')} +{:then value} + {console.log(value)} +{:catch} + {console.log('error')} +{/await} + + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js b/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js new file mode 100644 index 0000000000..11a645673a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js @@ -0,0 +1,27 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [b1, b2] = target.querySelectorAll('button'); + b1.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + `

then a

` + ); + b2.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + `

then a

` + ); + await Promise.resolve(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + `

then b

` + ); + + assert.deepEqual(logs, ['rendering pending block', 'rendering then block']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve/main.svelte b/packages/svelte/tests/runtime-runes/samples/await-resolve/main.svelte new file mode 100644 index 0000000000..62f2db7733 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve/main.svelte @@ -0,0 +1,17 @@ + + +{#await promise} + {console.log('rendering pending block')} +

pending

+{:then value} + {console.log('rendering then block')} +

then {value}

+{/await} + + + From be9b0a275af54ac55faf1ac29049587715ec26a4 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Thu, 20 Jun 2024 21:26:47 +0200 Subject: [PATCH 4/4] fix: repair each block length even without an else (#12098) * fix: repair each block length even without an else * chore: add changeset * simplify --------- Co-authored-by: Rich Harris --- .changeset/happy-lobsters-lick.md | 5 +++ .../src/internal/client/dom/blocks/each.js | 2 +- .../_config.js | 20 ++++++++++++ .../_expected.html | 9 ++++++ .../main.svelte | 32 +++++++++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 .changeset/happy-lobsters-lick.md create mode 100644 packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_config.js create mode 100644 packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_expected.html create mode 100644 packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/main.svelte diff --git a/.changeset/happy-lobsters-lick.md b/.changeset/happy-lobsters-lick.md new file mode 100644 index 0000000000..e4f68f04e7 --- /dev/null +++ b/.changeset/happy-lobsters-lick.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: repair each block length even without an else diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 0b6209c505..29d38ec621 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -151,7 +151,7 @@ export function each(anchor, flags, get_collection, get_key, render_fn, fallback if (hydrating) { var is_else = /** @type {Comment} */ (anchor).data === HYDRATION_END_ELSE; - if (is_else !== (length === 0)) { + if (is_else !== (length === 0) || hydrate_start === undefined) { // hydration mismatch — remove the server-rendered DOM and start over remove(hydrate_nodes); set_hydrating(false); diff --git a/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_config.js b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_config.js new file mode 100644 index 0000000000..edfcd70ca2 --- /dev/null +++ b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_config.js @@ -0,0 +1,20 @@ +import { assert_ok, test } from '../../test'; + +export default test({ + server_props: { + items: [] + }, + + props: { + items: [{ name: 'a' }] + }, + + snapshot(target) { + const ul = target.querySelector('ul'); + assert_ok(ul); + + return { + ul + }; + } +}); diff --git a/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_expected.html b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_expected.html new file mode 100644 index 0000000000..5de1a42d70 --- /dev/null +++ b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/_expected.html @@ -0,0 +1,9 @@ +
  • a
+
  • a
+
  • a
+
  • a
  • +
  • a
  • +
  • a
  • +
  • a
  • +
  • a
  • +
  • a
  • diff --git a/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/main.svelte b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/main.svelte new file mode 100644 index 0000000000..093e59cef9 --- /dev/null +++ b/packages/svelte/tests/hydration/samples/each-block-0-on-server-more-on-client/main.svelte @@ -0,0 +1,32 @@ + + +
      + {#each items as item} +
    • {item.name}
    • + {/each} +
    +
      + {#each items as item (item)} +
    • {item.name}
    • + {/each} +
    +
      + {#each items as item (item.name)} +
    • {item.name}
    • + {/each} +
    + +{#each items as item} +
  • {item.name}
  • +
  • {item.name}
  • +{/each} +{#each items as item (item)} +
  • {item.name}
  • +
  • {item.name}
  • +{/each} +{#each items as item (item.name)} +
  • {item.name}
  • +
  • {item.name}
  • +{/each}