From 7206a24919f91db62dfa89ab6971aa47204c1b2b Mon Sep 17 00:00:00 2001 From: raivaibhav Date: Tue, 19 Oct 2021 15:49:00 +0530 Subject: [PATCH 1/6] Fix: beforeUpdate called twice with bound reference Fixes: https://github.com/sveltejs/svelte/issues/6016 Fixes: https://github.com/sveltejs/svelte/issues/3290 --- src/runtime/internal/scheduler.ts | 19 ++++--- .../Item.svelte | 30 ++++++++++++ .../_config.js | 49 +++++++++++++++++++ .../main.svelte | 34 +++++++++++++ .../order.js | 1 + 5 files changed, 127 insertions(+), 6 deletions(-) create mode 100755 test/runtime/samples/lifecycle-render-order-for-children-with-binding/Item.svelte create mode 100644 test/runtime/samples/lifecycle-render-order-for-children-with-binding/_config.js create mode 100644 test/runtime/samples/lifecycle-render-order-for-children-with-binding/main.svelte create mode 100644 test/runtime/samples/lifecycle-render-order-for-children-with-binding/order.js diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 568739e4f862..61628c495da5 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -33,6 +33,8 @@ export function add_flush_callback(fn) { let flushing = false; const seen_callbacks = new Set(); +let dirty_binding_component_map = new Map(); + export function flush() { if (flushing) return; flushing = true; @@ -43,14 +45,19 @@ export function flush() { for (let i = 0; i < dirty_components.length; i += 1) { const component = dirty_components[i]; set_current_component(component); - update(component.$$); + const is_dirty_from_binding = dirty_binding_component_map.has(component.constructor.name); + update(component.$$, is_dirty_from_binding); } + dirty_binding_component_map = new Map(); set_current_component(null); dirty_components.length = 0; - while (binding_callbacks.length) binding_callbacks.pop()(); - + while (binding_callbacks.length) { + binding_callbacks.pop()(); + } + dirty_components.forEach((i) => + dirty_binding_component_map.set(i.constructor.name, i)); // then, once components are updated, call // afterUpdate functions. This may cause // subsequent updates... @@ -77,14 +84,14 @@ export function flush() { seen_callbacks.clear(); } -function update($$) { +function update($$, is_dirty_from_binding) { if ($$.fragment !== null) { $$.update(); - run_all($$.before_update); + if (!is_dirty_from_binding) run_all($$.before_update); const dirty = $$.dirty; $$.dirty = [-1]; $$.fragment && $$.fragment.p($$.ctx, dirty); - + // if (!is_dirty_from_binding) run_all($$.after_update); $$.after_update.forEach(add_render_callback); } } diff --git a/test/runtime/samples/lifecycle-render-order-for-children-with-binding/Item.svelte b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/Item.svelte new file mode 100755 index 000000000000..85d47d524520 --- /dev/null +++ b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/Item.svelte @@ -0,0 +1,30 @@ + + +
  • + {logRender()} +
    • diff --git a/test/runtime/samples/lifecycle-render-order-for-children-with-binding/_config.js b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/_config.js new file mode 100644 index 000000000000..32bc35d49caa --- /dev/null +++ b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/_config.js @@ -0,0 +1,49 @@ +import order from './order.js'; + +export default { + skip_if_ssr: true, + before_test() { + order.length = 0; + }, + test({ assert, compileOptions }) { + if (compileOptions.hydratable) { + assert.deepEqual(order, [ + '0: beforeUpdate', + '0: render', + '1: beforeUpdate', + '1: render', + '2: beforeUpdate', + '2: render', + '3: beforeUpdate', + '3: render', + '1: onMount', + '1: afterUpdate', + '2: onMount', + '2: afterUpdate', + '3: onMount', + '3: afterUpdate', + '0: onMount', + '0: afterUpdate' + ]); + } else { + assert.deepEqual(order, [ + '0: beforeUpdate', + '0: render', + '1: beforeUpdate', + '2: beforeUpdate', + '3: beforeUpdate', + '1: render', + '2: render', + '3: render', + '1: onMount', + '1: afterUpdate', + '2: onMount', + '2: afterUpdate', + '3: onMount', + '3: afterUpdate', + '0: onMount', + '0: afterUpdate' + ]); + } + } +}; diff --git a/test/runtime/samples/lifecycle-render-order-for-children-with-binding/main.svelte b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/main.svelte new file mode 100644 index 000000000000..c5061d32187d --- /dev/null +++ b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/main.svelte @@ -0,0 +1,34 @@ + + +{logRender()} + + + diff --git a/test/runtime/samples/lifecycle-render-order-for-children-with-binding/order.js b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/order.js new file mode 100644 index 000000000000..d6d1738de67e --- /dev/null +++ b/test/runtime/samples/lifecycle-render-order-for-children-with-binding/order.js @@ -0,0 +1 @@ +export default []; From e2941ac495d051dfc38a4aaa3a581354052018f1 Mon Sep 17 00:00:00 2001 From: vaibhav rai Date: Tue, 9 Nov 2021 22:26:48 +0530 Subject: [PATCH 2/6] Address initial review --- src/runtime/internal/scheduler.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 61628c495da5..57397edb767c 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -33,7 +33,7 @@ export function add_flush_callback(fn) { let flushing = false; const seen_callbacks = new Set(); -let dirty_binding_component_map = new Map(); +let previous_dirty_components = []; export function flush() { if (flushing) return; @@ -45,10 +45,10 @@ export function flush() { for (let i = 0; i < dirty_components.length; i += 1) { const component = dirty_components[i]; set_current_component(component); - const is_dirty_from_binding = dirty_binding_component_map.has(component.constructor.name); + const is_dirty_from_binding = previous_dirty_components.indexOf(component) > -1; update(component.$$, is_dirty_from_binding); } - dirty_binding_component_map = new Map(); + previous_dirty_components = []; set_current_component(null); dirty_components.length = 0; @@ -56,8 +56,7 @@ export function flush() { while (binding_callbacks.length) { binding_callbacks.pop()(); } - dirty_components.forEach((i) => - dirty_binding_component_map.set(i.constructor.name, i)); + previous_dirty_components = [...dirty_components] // then, once components are updated, call // afterUpdate functions. This may cause // subsequent updates... From ef2c3b2d88f92cc462534aaed1e552bc69765bbf Mon Sep 17 00:00:00 2001 From: vaibhav rai Date: Tue, 9 Nov 2021 22:30:25 +0530 Subject: [PATCH 3/6] make variable name more relatable --- src/runtime/internal/scheduler.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 57397edb767c..3f28527cb277 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -45,8 +45,8 @@ export function flush() { for (let i = 0; i < dirty_components.length; i += 1) { const component = dirty_components[i]; set_current_component(component); - const is_dirty_from_binding = previous_dirty_components.indexOf(component) > -1; - update(component.$$, is_dirty_from_binding); + const is_previous_dirty = previous_dirty_components.indexOf(component) > -1; + update(component.$$, is_previous_dirty); } previous_dirty_components = []; set_current_component(null); @@ -83,14 +83,14 @@ export function flush() { seen_callbacks.clear(); } -function update($$, is_dirty_from_binding) { +function update($$, is_previous_dirty) { if ($$.fragment !== null) { $$.update(); - if (!is_dirty_from_binding) run_all($$.before_update); + if (!is_previous_dirty) run_all($$.before_update); const dirty = $$.dirty; $$.dirty = [-1]; $$.fragment && $$.fragment.p($$.ctx, dirty); - // if (!is_dirty_from_binding) run_all($$.after_update); + // if (!is_previous_dirty) run_all($$.after_update); $$.after_update.forEach(add_render_callback); } } From fa7b1d4839126b4a6fc3513f85496872e094af25 Mon Sep 17 00:00:00 2001 From: vaibhav rai Date: Tue, 9 Nov 2021 23:38:02 +0530 Subject: [PATCH 4/6] fix lint and rebase --- src/runtime/internal/scheduler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 3f28527cb277..3f1403b081f4 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -56,7 +56,7 @@ export function flush() { while (binding_callbacks.length) { binding_callbacks.pop()(); } - previous_dirty_components = [...dirty_components] + previous_dirty_components = [...dirty_components]; // then, once components are updated, call // afterUpdate functions. This may cause // subsequent updates... From 6a948ee09df7571862f7bd0dba9eb7175b83cea9 Mon Sep 17 00:00:00 2001 From: vaibhav rai Date: Wed, 10 Nov 2021 20:31:56 +0530 Subject: [PATCH 5/6] Convert to set object --- src/runtime/internal/scheduler.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 3f1403b081f4..13039e500352 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -33,7 +33,7 @@ export function add_flush_callback(fn) { let flushing = false; const seen_callbacks = new Set(); -let previous_dirty_components = []; +let previous_dirty_components = new Set(); export function flush() { if (flushing) return; @@ -45,10 +45,10 @@ export function flush() { for (let i = 0; i < dirty_components.length; i += 1) { const component = dirty_components[i]; set_current_component(component); - const is_previous_dirty = previous_dirty_components.indexOf(component) > -1; + const is_previous_dirty = previous_dirty_components.has(component); update(component.$$, is_previous_dirty); } - previous_dirty_components = []; + set_current_component(null); dirty_components.length = 0; @@ -56,7 +56,7 @@ export function flush() { while (binding_callbacks.length) { binding_callbacks.pop()(); } - previous_dirty_components = [...dirty_components]; + previous_dirty_components = new Set(dirty_components); // then, once components are updated, call // afterUpdate functions. This may cause // subsequent updates... @@ -81,6 +81,7 @@ export function flush() { update_scheduled = false; flushing = false; seen_callbacks.clear(); + previous_dirty_components.clear(); } function update($$, is_previous_dirty) { From 463038f50dfde20433fd9b9389b40cfcff848199 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 14 Mar 2023 15:56:57 +0100 Subject: [PATCH 6/6] Update src/runtime/internal/scheduler.ts --- src/runtime/internal/scheduler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/internal/scheduler.ts b/src/runtime/internal/scheduler.ts index 60247f25d8e8..986d69b87f8d 100644 --- a/src/runtime/internal/scheduler.ts +++ b/src/runtime/internal/scheduler.ts @@ -75,6 +75,7 @@ export function flush() { } } catch (e) { // reset dirty state to not end up in a deadlocked state and then rethrow + previous_dirty_components.clear(); dirty_components.length = 0; flushidx = 0; throw e;