From a69c3743810526e31e0a85e4362642357a7a5846 Mon Sep 17 00:00:00 2001 From: subtle-byte Date: Sun, 16 May 2021 05:32:52 +0300 Subject: [PATCH 1/4] Fixing reactivity glitches --- src/runtime/store/index.ts | 58 +++++++++++++++++++++++++------------- test/store/index.js | 45 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 20 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index cf15db5250b9..6965da68848b 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -13,7 +13,7 @@ export type Updater = (value: T) => T; type Invalidator = (value?: T) => void; /** Start and stop notification callbacks. */ -export type StartStopNotifier = (set: Subscriber) => Unsubscriber | void; +export type StartStopNotifier = (set: Subscriber, invalidate: Invalidator) => Unsubscriber | void; /** Readable interface for subscribing. */ export interface Readable { @@ -22,7 +22,7 @@ export interface Readable { * @param run subscription callback * @param invalidate cleanup callback */ - subscribe(this: void, run: Subscriber, invalidate?: Invalidator): Unsubscriber; + subscribe(this: void, run: Subscriber, invalidate?: Invalidator, revalidate?: Revalidator): Unsubscriber; } /** Writable interface for both updating and subscribing. */ @@ -40,8 +40,9 @@ export interface Writable extends Readable { update(this: void, updater: Updater): void; } -/** Pair of subscriber and invalidator. */ -type SubscribeInvalidateTuple = [Subscriber, Invalidator]; +type Revalidator = () => void; +/** Tuple of subscriber, invalidator, and revalidator. */ +type Subscription = [Subscriber, Invalidator, Revalidator]; const subscriber_queue = []; @@ -63,17 +64,27 @@ export function readable(value: T, start: StartStopNotifier): Readable */ export function writable(value: T, start: StartStopNotifier = noop): Writable { let stop: Unsubscriber; - const subscribers: Array> = []; + const subscriptions: Array> = []; + + function invalidate() { + for (let i = 0; i < subscriptions.length; i += 1) { + subscriptions[i][1](); + } + } + function revalidate() { + for (let i = 0; i < subscriptions.length; i += 1) { + subscriptions[i][2](); + } + } function set(new_value: T): void { if (safe_not_equal(value, new_value)) { value = new_value; if (stop) { // store is ready const run_queue = !subscriber_queue.length; - for (let i = 0; i < subscribers.length; i += 1) { - const s = subscribers[i]; - s[1](); - subscriber_queue.push(s, value); + invalidate(); + for (let i = 0; i < subscriptions.length; i += 1) { + subscriber_queue.push(subscriptions[i], value); } if (run_queue) { for (let i = 0; i < subscriber_queue.length; i += 2) { @@ -82,6 +93,8 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa subscriber_queue.length = 0; } } + } else { + revalidate(); } } @@ -89,20 +102,20 @@ export function writable(value: T, start: StartStopNotifier = noop): Writa set(fn(value)); } - function subscribe(run: Subscriber, invalidate: Invalidator = noop): Unsubscriber { - const subscriber: SubscribeInvalidateTuple = [run, invalidate]; - subscribers.push(subscriber); - if (subscribers.length === 1) { - stop = start(set) || noop; + function subscribe(run: Subscriber, invalidate: Invalidator = noop, revalidate: Revalidator = noop): Unsubscriber { + const subscription: Subscription = [run, invalidate, revalidate]; + subscriptions.push(subscription); + if (subscriptions.length === 1) { + stop = start(set, invalidate) || noop; } run(value); return () => { - const index = subscribers.indexOf(subscriber); + const index = subscriptions.indexOf(subscription); if (index !== -1) { - subscribers.splice(index, 1); + subscriptions.splice(index, 1); } - if (subscribers.length === 0) { + if (subscriptions.length === 0) { stop(); stop = null; } @@ -167,7 +180,7 @@ export function derived(stores: Stores, fn: Function, initial_value?: T): Rea const auto = fn.length < 2; - return readable(initial_value, (set) => { + return readable(initial_value, (set, invalidate) => { let inited = false; const values = []; @@ -197,9 +210,14 @@ export function derived(stores: Stores, fn: Function, initial_value?: T): Rea } }, () => { + const invalidated = pending & (1 << i); pending |= (1 << i); - }) - ); + if (!invalidated) { + invalidate(); + } + }, + () => pending &= ~(1 << i) + )); inited = true; sync(); diff --git a/test/store/index.js b/test/store/index.js index 2af4a6f35d85..9ed974a67cf9 100644 --- a/test/store/index.js +++ b/test/store/index.js @@ -214,6 +214,51 @@ describe('store', () => { unsubscribe(); }); + it('prevents glitches 2', () => { + const lastname = writable('Jekyll'); + const firstname = derived(lastname, n => n === 'Jekyll' ? 'Henry' : 'Edward'); + + const fullname = derived([lastname, firstname], names => names.reverse().join(' ')); + + const values = []; + + const unsubscribe = fullname.subscribe(value => { + values.push(value); + }); + + lastname.set('Hyde'); + + assert.deepEqual(values, [ + 'Henry Jekyll', + 'Edward Hyde' + ]); + + unsubscribe(); + }); + + it('prevents glitches 3', () => { + const lastname = writable('Jekyll'); + const firstname_first_letter = derived(lastname, n => n === 'Jekyll' ? 'H' : 'E'); + const firstname = derived(firstname_first_letter, n => n === 'H' ? 'Henry' : 'Edward'); + + const fullname = derived([lastname, firstname], names => names.reverse().join(' ')); + + const values = []; + + const unsubscribe = fullname.subscribe(value => { + values.push(value); + }); + + lastname.set('Hyde'); + + assert.deepEqual(values, [ + 'Henry Jekyll', + 'Edward Hyde' + ]); + + unsubscribe(); + }); + it('prevents diamond dependency problem', () => { const count = writable(0); const values = []; From 53607d57cc7b61b2573a08823b41bb55bbd773c2 Mon Sep 17 00:00:00 2001 From: subtle-byte Date: Fri, 21 May 2021 03:43:20 +0300 Subject: [PATCH 2/4] More correct algorithm --- src/runtime/store/index.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index 8e6f0ba0841c..f4816a98c0a5 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -13,7 +13,7 @@ export type Updater = (value: T) => T; type Invalidator = (value?: T) => void; /** Start and stop notification callbacks. */ -export type StartStopNotifier = (set: Subscriber, invalidate: Invalidator) => Unsubscriber | void; +export type StartStopNotifier = (set: Subscriber, invalidate: Invalidator, revalidate: Revalidator) => Unsubscriber | void; /** Readable interface for subscribing. */ export interface Readable { @@ -106,7 +106,7 @@ export function writable(value?: T, start: StartStopNotifier = noop): Writ const subscription: Subscription = [run, invalidate, revalidate]; subscriptions.push(subscription); if (subscriptions.length === 1) { - stop = start(set, invalidate) || noop; + stop = start(set, invalidate, revalidate) || noop; } run(value); @@ -180,7 +180,7 @@ export function derived(stores: Stores, fn: Function, initial_value?: T): Rea const auto = fn.length < 2; - return readable(initial_value, (set, invalidate) => { + return readable(initial_value, (set, invalidate, revalidate) => { let inited = false; const values = []; @@ -210,13 +210,18 @@ export function derived(stores: Stores, fn: Function, initial_value?: T): Rea } }, () => { - const invalidated = pending & (1 << i); + const invalidated = pending; pending |= (1 << i); if (!invalidated) { invalidate(); } }, - () => pending &= ~(1 << i) + () => { + pending &= ~(1 << i) + if (!pending) { + revalidate(); + } + }, )); inited = true; From d18baa418e9dc0b1411edf414884d0dce2aed7ef Mon Sep 17 00:00:00 2001 From: subtle-byte Date: Fri, 21 May 2021 03:52:25 +0300 Subject: [PATCH 3/4] Linting the project --- src/runtime/store/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index f4816a98c0a5..a6c31299e8a0 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -217,11 +217,11 @@ export function derived(stores: Stores, fn: Function, initial_value?: T): Rea } }, () => { - pending &= ~(1 << i) + pending &= ~(1 << i); if (!pending) { revalidate(); } - }, + } )); inited = true; From 2254311218b23fc025d27d867372bcbc86344010 Mon Sep 17 00:00:00 2001 From: subtle-byte Date: Wed, 28 Jul 2021 02:04:48 +0300 Subject: [PATCH 4/4] Refactoring + fixing the bug related to shadowing of function --- src/runtime/store/index.ts | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/runtime/store/index.ts b/src/runtime/store/index.ts index 4b8541ac76f5..e16997daea6c 100644 --- a/src/runtime/store/index.ts +++ b/src/runtime/store/index.ts @@ -9,20 +9,26 @@ export type Unsubscriber = () => void; /** Callback to update a value. */ export type Updater = (value: T) => T; -/** Cleanup logic callback. */ -type Invalidator = (value?: T) => void; +/** + * Callback to inform that the store has an invalid value. + * + * "Invalid" means that a dependency's value has been updated, + * so this store's value should be updated. +*/ +type Invalidator = () => void; +/** Callback to inform that the store has a valid value. */ +type Revalidator = () => void; /** Start and stop notification callbacks. */ -export type StartStopNotifier = (set: Subscriber, invalidate: Invalidator, revalidate: Revalidator) => Unsubscriber | void; +export type StartStopNotifier = (set: Subscriber, invalidate: Invalidator, revalidate: Revalidator) => Unsubscriber | void; /** Readable interface for subscribing. */ export interface Readable { /** * Subscribe on value changes. * @param run subscription callback - * @param invalidate cleanup callback */ - subscribe(this: void, run: Subscriber, invalidate?: Invalidator, revalidate?: Revalidator): Unsubscriber; + subscribe(this: void, run: Subscriber, invalidate?: Invalidator, revalidate?: Revalidator): Unsubscriber; } /** Writable interface for both updating and subscribing. */ @@ -40,9 +46,7 @@ export interface Writable extends Readable { update(this: void, updater: Updater): void; } -type Revalidator = () => void; -/** Tuple of subscriber, invalidator, and revalidator. */ -type Subscription = [Subscriber, Invalidator, Revalidator]; +type Subscription = [Subscriber, Invalidator, Revalidator]; const subscriber_queue = []; @@ -66,12 +70,14 @@ export function writable(value?: T, start: StartStopNotifier = noop): Writ let stop: Unsubscriber; const subscriptions: Set> = new Set(); - function invalidate() { + function invalidate_writable() { + // Inform all subscribers that this store has an invalid value for (const subscription of subscriptions) { subscription[1](); } } - function revalidate() { + function revalidate_writable() { + // Inform all subscribers that this store has a valid value for (const subscription of subscriptions) { subscription[2](); } @@ -82,7 +88,7 @@ export function writable(value?: T, start: StartStopNotifier = noop): Writ value = new_value; if (stop) { // store is ready const run_queue = !subscriber_queue.length; - invalidate(); + invalidate_writable(); for (const subscription of subscriptions) { subscriber_queue.push(subscription, value); } @@ -94,7 +100,7 @@ export function writable(value?: T, start: StartStopNotifier = noop): Writ } } } else { - revalidate(); + revalidate_writable(); } } @@ -102,11 +108,11 @@ export function writable(value?: T, start: StartStopNotifier = noop): Writ set(fn(value)); } - function subscribe(run: Subscriber, invalidate: Invalidator = noop, revalidate: Revalidator = noop): Unsubscriber { + function subscribe(run: Subscriber, invalidate: Invalidator = noop, revalidate: Revalidator = noop): Unsubscriber { const subscription: Subscription = [run, invalidate, revalidate]; subscriptions.add(subscription); if (subscriptions.size === 1) { - stop = start(set, invalidate, revalidate) || noop; + stop = start(set, invalidate_writable, revalidate_writable) || noop; } run(value);