Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Derived stores are re-evaluated on invalid upstream state #10557

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions packages/svelte/src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,48 @@ export function safe_not_equal(a, b) {
export function writable(value, start = noop) {
/** @type {import('./public').Unsubscriber | null} */
let stop = null;
let invalidated = false;

/** @type {Set<import('./private').SubscribeInvalidateTuple<T>>} */
const subscribers = new Set();

function invalidate() {
if (invalidated)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use early return pattern

Suggested change
if (invalidated)
if (invalidated || !stop) return;
invalidated = true;
// immediately signal each subscriber as invalid
for (const subscriber of subscribers) {
subscriber[1]();
}

return;

if (stop) {
invalidated = true;

// immediately signal each subscriber as invalid
for (const subscriber of subscribers) {
subscriber[1]();
}
}
}

function revalidate() {
if (!invalidated)
return;

invalidated = false;

if (stop) {
// immediately signal each subscriber as revalidated
for (const subscriber of subscribers) {
subscriber[2]();
}
}
}

/**
* @param {T} new_value
* @returns {void}
*/
function set(new_value) {
if (safe_not_equal(value, new_value)) {
value = new_value;
invalidated = false;

if (stop) {
// store is ready
const run_queue = !subscriber_queue.length;
Expand All @@ -72,8 +103,25 @@ export function writable(value, start = noop) {
}
}
}
else
revalidate();
}


const complex_set = Object.assign(
/**
* @param {T} new_value
* @returns {void}
*/
(new_value) => set(new_value),
{
set,
update,
invalidate,
revalidate
}
)

/**
* @param {import('./public').Updater<T>} fn
* @returns {void}
Expand All @@ -85,14 +133,15 @@ export function writable(value, start = noop) {
/**
* @param {import('./public').Subscriber<T>} run
* @param {import('./private').Invalidator<T>} [invalidate]
* @param {import('./private').Revalidator<T>} [revalidate]
* @returns {import('./public').Unsubscriber}
*/
function subscribe(run, invalidate = noop) {
function subscribe(run, invalidate = noop, revalidate = noop) {
/** @type {import('./private').SubscribeInvalidateTuple<T>} */
const subscriber = [run, invalidate];
const subscriber = [run, invalidate, revalidate];
subscribers.add(subscriber);
if (subscribers.size === 1) {
stop = start(set, update) || noop;
stop = start(complex_set, update) || noop;
}
run(/** @type {T} */ (value));
return () => {
Expand Down Expand Up @@ -157,15 +206,18 @@ export function derived(stores, fn, initial_value) {
}
const auto = fn.length < 2;
return readable(initial_value, (set, update) => {
const { invalidate, revalidate } = set;
let started = false;
/** @type {T[]} */
const values = [];
let pending = 0;
let changed = stores_array.length === 0;
let cleanup = noop;
const sync = () => {
if (pending) {
if (!changed || pending) {
return;
}
changed = false;
cleanup();
const result = fn(single ? values[0] : values, set, update);
if (auto) {
Expand All @@ -179,13 +231,25 @@ export function derived(stores, fn, initial_value) {
store,
(value) => {
values[i] = value;
changed = true;
pending &= ~(1 << i);
if (started) {
sync();
}
},
() => {
pending |= 1 << i;
invalidate();
},
() => {
pending &= ~(1 << i);
if (!changed && !pending) {
revalidate();
}
else
if (started) {
sync();
}
}
)
);
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/store/private.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import { Readable, Subscriber } from './public.js';
/** Cleanup logic callback. */
export type Invalidator<T> = (value?: T) => void;

/** Cleanup logic callback. */
export type Revalidator<T> = (value?: T) => void;

/** Pair of subscriber and invalidator. */
export type SubscribeInvalidateTuple<T> = [Subscriber<T>, Invalidator<T>];
export type SubscribeInvalidateTuple<T> = [Subscriber<T>, Invalidator<T>, Revalidator<T>];

/** One or more `Readable`s. */
export type Stores =
Expand Down
16 changes: 11 additions & 5 deletions packages/svelte/src/store/public.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Invalidator } from './private.js';
import type { Invalidator, Revalidator } from './private.js';

/** Callback to inform of a value updates. */
export type Subscriber<T> = (value: T) => void;
Expand All @@ -13,13 +13,18 @@ export type Updater<T> = (value: T) => T;
* Start and stop notification callbacks.
* This function is called when the first subscriber subscribes.
*
* @param {(value: T) => void} set Function that sets the value of the store.
* @param {((value: T) => void) & { set: (value: T) => void, update: (fn: Updater<T>) => void, invalidate: () => void })} set Function that sets the value of the store.
* @param {(value: Updater<T>) => void} update Function that sets the value of the store after passing the current value to the update function.
* @returns {void | (() => void)} Optionally, a cleanup function that is called when the last remaining
* subscriber unsubscribes.
*/
export type StartStopNotifier<T> = (
set: (value: T) => void,
set: ((value: T) => void) & {
set: (value: T) => void,
update: (fn: Updater<T>) => void,
invalidate: () => void,
revalidate: () => void
},
update: (fn: Updater<T>) => void
) => void | (() => void);

Expand All @@ -28,9 +33,10 @@ export interface Readable<T> {
/**
* Subscribe on value changes.
* @param run subscription callback
* @param invalidate cleanup callback
* @param invalidate cleanup callback - run when inputs are in an indeterminate state
* @param revalidate cleanup callback - run when inputs have been resolved to their previous values
*/
subscribe(this: void, run: Subscriber<T>, invalidate?: Invalidator<T>): Unsubscriber;
subscribe(this: void, run: Subscriber<T>, invalidate?: Invalidator<T>, revalidate?: Revalidator<T>): Unsubscriber;
}

/** Writable interface for both updating and subscribing. */
Expand Down
6 changes: 4 additions & 2 deletions packages/svelte/src/store/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import { noop } from '../internal/common.js';
* @param {import('./public').Readable<T> | null | undefined} store
* @param {(value: T) => void} run
* @param {(value: T) => void} [invalidate]
* @param {(value: T) => void} [revalidate]
* @returns {() => void}
*/
export function subscribe_to_store(store, run, invalidate) {
export function subscribe_to_store(store, run, invalidate, revalidate) {
if (store == null) {
// @ts-expect-error
run(undefined);
Expand All @@ -22,7 +23,8 @@ export function subscribe_to_store(store, run, invalidate) {
const unsub = store.subscribe(
run,
// @ts-expect-error
invalidate
invalidate,
revalidate
);

// Also support RxJS
Expand Down
16 changes: 16 additions & 0 deletions packages/svelte/tests/store/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,22 @@ describe('derived', () => {
});
});
});

it('only updates once dependents are resolved', () => {
const a = writable(1);
const b = derived(a, a => a*2);
const c = derived([a,b], ([a,b]) => a+b);

const values: number[] = [];

const unsubscribe = c.subscribe(c => {
values.push(c);
});

a.set(2);
a.set(3);
assert.deepEqual(values, [3, 6, 9]);
});
});

describe('get', () => {
Expand Down