From bbeafbafab0e10e4f88f898890d97f5e508f5609 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Thu, 11 Jul 2019 01:16:43 -0400 Subject: [PATCH 1/5] fixes https://github.com/sveltejs/svelte/issues/3191 Derived store reruns subscribers if it's value has not changed when synced. All invalidators of subscribers are run on a derived store when invalidated. See https://github.com/sveltejs/svelte/pull/2955 --- src/runtime/store/index.ts | 19 +++++++++++++------ test/js/samples/bind-open/expected.js | 2 +- test/store/index.ts | 24 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index c85163003f7a..c5bbb052cf4a 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -67,16 +67,15 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa function set(new_value: T): void { if (safe_not_equal(value, new_value)) { value = new_value; - if (!stop) { - return; // not ready + if (stop) { // store is ready + subscribers.forEach((s) => s[1]()); + subscribers.forEach((s) => s[0](value)); } - subscribers.forEach((s) => s[1]()); - subscribers.forEach((s) => s[0](value)); } } function update(fn: Updater): void { - set(fn(value)); + return set(fn(value)); } function subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { @@ -129,7 +128,9 @@ export function derived( const auto = fn.length < 2; + const subscribers: Array> = []; const invalidators: Array> = []; + let value: T = initial_value; const store = readable(initial_value, (set) => { let inited = false; @@ -146,6 +147,11 @@ export function derived( const result = fn(single ? values[0] : values, set); if (auto) { set(result as T); + const dirty = safe_not_equal(value, result); + value = result as T; + if (!dirty) { + subscribers.forEach(s => s(value)); + } } else { cleanup = is_function(result) ? result as Unsubscriber : noop; } @@ -176,6 +182,7 @@ export function derived( return { subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { + subscribers.push(run); invalidators.push(invalidate); const unsubscribe = store.subscribe(run, invalidate); @@ -189,4 +196,4 @@ export function derived( }; } }; -} \ No newline at end of file +} diff --git a/test/js/samples/bind-open/expected.js b/test/js/samples/bind-open/expected.js index 7f739aec8b72..7c73c8ddac3c 100644 --- a/test/js/samples/bind-open/expected.js +++ b/test/js/samples/bind-open/expected.js @@ -66,4 +66,4 @@ class Component extends SvelteComponent { } } -export default Component; \ No newline at end of file +export default Component; diff --git a/test/store/index.ts b/test/store/index.ts index 6f5ef6abdd5e..a39fab86e6e7 100644 --- a/test/store/index.ts +++ b/test/store/index.ts @@ -233,6 +233,30 @@ describe('store', () => { unsubscribe(); }); + it('derived dependency does not update and shared ancestor updates', () => { + const root = writable({ a: 0, b:0 }); + const values = []; + + const a = derived(root, $root => { + return 'a' + $root.a; + }); + + const b = derived([a, root], ([$a, $root]) => { + return 'b' + $root.b + $a; + }); + + const unsubscribe = b.subscribe(v => { + values.push(v); + }); + + assert.deepEqual(values, ['b0a0']); + + root.set({ a: 0, b: 1 }); + assert.deepEqual(values, ['b0a0', 'b1a0']); + + unsubscribe(); + }); + it('is updated with safe_not_equal logic', () => { const arr = [0]; From 46c9dbe4482ef189157cb59c0fe6dfe081812d96 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Thu, 11 Jul 2019 03:14:29 -0400 Subject: [PATCH 2/5] Breadth first calling of subscribers in store dependency tree. Optimize performance by using subscriber_queue. --- src/runtime/store/index.ts | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index c5bbb052cf4a..25c37945c63f 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -44,6 +44,8 @@ export interface Writable extends Readable { /** Pair of subscriber and invalidator. */ type SubscribeInvalidateTuple = [Subscriber, Invalidator]; +const subscriber_queue = []; + /** * Creates a `Readable` store that allows reading by subscription. * @param value initial value @@ -69,7 +71,15 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa value = new_value; if (stop) { // store is ready subscribers.forEach((s) => s[1]()); - subscribers.forEach((s) => s[0](value)); + const run_queue = !subscriber_queue.length; + subscribers.forEach(s => subscriber_queue.push(s, value)); + if (run_queue) { + let s; + while (s = subscriber_queue.shift()) { + const val = subscriber_queue.shift(); + s[0](val); + } + } } } } @@ -128,10 +138,6 @@ export function derived( const auto = fn.length < 2; - const subscribers: Array> = []; - const invalidators: Array> = []; - let value: T = initial_value; - const store = readable(initial_value, (set) => { let inited = false; const values: StoresValues = [] as StoresValues; @@ -147,11 +153,6 @@ export function derived( const result = fn(single ? values[0] : values, set); if (auto) { set(result as T); - const dirty = safe_not_equal(value, result); - value = result as T; - if (!dirty) { - subscribers.forEach(s => s(value)); - } } else { cleanup = is_function(result) ? result as Unsubscriber : noop; } @@ -166,7 +167,6 @@ export function derived( } }, () => { - run_all(invalidators); pending |= (1 << i); }), ); @@ -182,18 +182,7 @@ export function derived( return { subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { - subscribers.push(run); - invalidators.push(invalidate); - - const unsubscribe = store.subscribe(run, invalidate); - - return () => { - const index = invalidators.indexOf(invalidate); - if (index !== -1) { - invalidators.splice(index, 1); - } - unsubscribe(); - }; + return store.subscribe(run, invalidate); } }; } From 2b0a4e7237dc1e1317c00adeacbdf75ee3d259c3 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Thu, 11 Jul 2019 03:30:35 -0400 Subject: [PATCH 3/5] remove unnecessary return --- src/runtime/store/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index 25c37945c63f..98761b847fe3 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -85,7 +85,7 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa } function update(fn: Updater): void { - return set(fn(value)); + set(fn(value)); } function subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { From 419e40d3be1fceced7015ea59708015c8d70108d Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Thu, 11 Jul 2019 03:42:54 -0400 Subject: [PATCH 4/5] Optimization: Single for loop instead of 2 forEach loops --- src/runtime/store/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index 98761b847fe3..7f94351437e0 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -70,9 +70,12 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa if (safe_not_equal(value, new_value)) { value = new_value; if (stop) { // store is ready - subscribers.forEach((s) => s[1]()); const run_queue = !subscriber_queue.length; - subscribers.forEach(s => subscriber_queue.push(s, value)); + for (let i = 0; i < subscribers.length; i++) { + const s = subscribers[i]; + s[1](); + subscriber_queue.push(s, value); + } if (run_queue) { let s; while (s = subscriber_queue.shift()) { From 95b2a72007c47b0225ec06decdc62296b80b131e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 26 Jul 2019 08:38:35 -0400 Subject: [PATCH 5/5] return readable store directly form derived, avoid .shift() --- src/runtime/store/index.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index 7f94351437e0..6863730e4e98 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -71,17 +71,16 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa value = new_value; if (stop) { // store is ready const run_queue = !subscriber_queue.length; - for (let i = 0; i < subscribers.length; i++) { + for (let i = 0; i < subscribers.length; i += 1) { const s = subscribers[i]; s[1](); subscriber_queue.push(s, value); } if (run_queue) { - let s; - while (s = subscriber_queue.shift()) { - const val = subscriber_queue.shift(); - s[0](val); + for (let i = 0; i < subscriber_queue.length; i += 2) { + subscriber_queue[i][0](subscriber_queue[i + 1]); } + subscriber_queue.length = 0; } } } @@ -141,7 +140,7 @@ export function derived( const auto = fn.length < 2; - const store = readable(initial_value, (set) => { + return readable(initial_value, (set) => { let inited = false; const values: StoresValues = [] as StoresValues; @@ -182,10 +181,4 @@ export function derived( cleanup(); }; }); - - return { - subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { - return store.subscribe(run, invalidate); - } - }; }