From fb4313ea31924b90e4cd4a560f92084511648784 Mon Sep 17 00:00:00 2001 From: mnrx <83848843+mnrx@users.noreply.github.com> Date: Wed, 15 Nov 2023 05:22:11 +0000 Subject: [PATCH 1/5] Tidy up store tests --- packages/svelte/tests/store/test.ts | 59 ++++++++++++++++------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/packages/svelte/tests/store/test.ts b/packages/svelte/tests/store/test.ts index 7c1e5b8f6817..c60b9c687ea8 100644 --- a/packages/svelte/tests/store/test.ts +++ b/packages/svelte/tests/store/test.ts @@ -22,7 +22,7 @@ describe('writable', () => { assert.deepEqual(values, [0, 1, 2]); }); - it('creates an undefined writable store', () => { + it('creates an `undefined` writable store', () => { const store = writable(); const values: unknown[] = []; @@ -41,7 +41,9 @@ describe('writable', () => { const store = writable(0, () => { called += 1; - return () => (called -= 1); + return () => { + called -= 1; + }; }); const unsubscribe1 = store.subscribe(() => {}); @@ -81,16 +83,20 @@ describe('writable', () => { let count1 = 0; let count2 = 0; - store.subscribe(() => (count1 += 1))(); + store.subscribe(() => { + count1 += 1; + })(); assert.equal(count1, 1); - const unsubscribe = store.subscribe(() => (count2 += 1)); + const unsubscribe = store.subscribe(() => { + count2 += 1; + }); assert.equal(count2, 1); unsubscribe(); }); - it('no error even if unsubscribe calls twice', () => { + it('does not throw an error if `unsubscribe` is called more than once', () => { let num = 0; const store = writable(num, (set) => set((num += 1))); const unsubscribe = store.subscribe(() => {}); @@ -139,7 +145,7 @@ describe('readable', () => { assert.deepEqual(values, [0, 1, 2]); }); - it('passes an optional update function', () => { + it('passes an optional `update` function', () => { let running; let tick = (value: any) => {}; @@ -186,7 +192,7 @@ describe('readable', () => { assert.deepEqual(values, [0, 1, 2, 5, 9, 5, 11]); }); - it('creates an undefined readable store', () => { + it('creates an `undefined` readable store', () => { const store = readable(); const values: unknown[] = []; @@ -270,7 +276,7 @@ describe('derived', () => { assert.deepEqual(values, [6, 12, 20]); }); - it('passes optional set function', () => { + it('passes optional `set` function', () => { const number = writable(1); const evens = derived( number, @@ -301,9 +307,9 @@ describe('derived', () => { assert.deepEqual(values, [0, 2, 4]); }); - it('passes optional set and update functions', () => { + it('passes optional `set` and `update` functions', () => { const number = writable(1); - const evensAndSquaresOf4 = derived( + const evens_and_squares_of_4 = derived( number, // @ts-expect-error TODO feels like inference should work here (n, set, update) => { @@ -316,7 +322,7 @@ describe('derived', () => { const values: number[] = []; - const unsubscribe = evensAndSquaresOf4.subscribe((value) => { + const unsubscribe = evens_and_squares_of_4.subscribe((value) => { values.push(value); }); @@ -341,21 +347,21 @@ describe('derived', () => { }); it('prevents glitches', () => { - const lastname = writable('Jekyll'); + const last_name = writable('Jekyll'); // @ts-expect-error TODO feels like inference should work here - const firstname = derived(lastname, (n) => (n === 'Jekyll' ? 'Henry' : 'Edward')); + const first_name = derived(last_name, (n) => (n === 'Jekyll' ? 'Henry' : 'Edward')); // @ts-expect-error TODO feels like inference should work here - const fullname = derived([firstname, lastname], (names) => names.join(' ')); + const full_name = derived([first_name, last_name], (names) => names.join(' ')); const values: string[] = []; - const unsubscribe = fullname.subscribe((value) => { + const unsubscribe = full_name.subscribe((value) => { values.push(value as string); }); - lastname.set('Hyde'); + last_name.set('Hyde'); assert.deepEqual(values, ['Henry Jekyll', 'Edward Hyde']); @@ -364,7 +370,6 @@ describe('derived', () => { it('prevents diamond dependency problem', () => { const count = writable(0); - const values: string[] = []; // @ts-expect-error TODO feels like inference should work here @@ -421,7 +426,7 @@ describe('derived', () => { unsubscribe(); }); - it('is updated with safe_not_equal logic', () => { + it('is updated with `safe_not_equal` logic', () => { const arr = [0]; const number = writable(1); @@ -446,7 +451,7 @@ describe('derived', () => { unsubscribe(); }); - it('calls a cleanup function', () => { + it('calls a `cleanup` function', () => { const num = writable(1); const values: number[] = []; @@ -523,7 +528,7 @@ describe('derived', () => { assert.equal(get(d), 42); }); - it("doesn't restart when unsubscribed from another store with a shared ancestor", () => { + it('does not restart when unsubscribed from another store with a shared ancestor', () => { const a = writable(true); let b_started = false; @@ -577,17 +582,17 @@ describe('get', () => { describe('readonly', () => { it('makes a store readonly', () => { - const writableStore = writable(1); - const readableStore = readonly(writableStore); + const writable_store = writable(1); + const readable_store = readonly(writable_store); - assert.equal(get(readableStore), get(writableStore)); + assert.equal(get(readable_store), get(writable_store)); - writableStore.set(2); + writable_store.set(2); - assert.equal(get(readableStore), 2); - assert.equal(get(readableStore), get(writableStore)); + assert.equal(get(readable_store), 2); + assert.equal(get(readable_store), get(writable_store)); // @ts-ignore - assert.throws(() => readableStore.set(3)); + assert.throws(() => readable_store.set(3)); }); }); From 25e7880b78a1cc75e62865fc23752827af0470d0 Mon Sep 17 00:00:00 2001 From: mnrx <83848843+mnrx@users.noreply.github.com> Date: Wed, 15 Nov 2023 05:24:29 +0000 Subject: [PATCH 2/5] Fix bug in `tweened()` and re-add tests --- packages/svelte/src/motion/spring.js | 5 +++- packages/svelte/src/motion/tweened.js | 9 ++++--- packages/svelte/tests/motion/test.ts | 38 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 packages/svelte/tests/motion/test.ts diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index 09d11402d5da..01fb06e8a203 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -48,7 +48,10 @@ function tick_spring(ctx, last_value, current_value, target_value) { } /** - * The spring function in Svelte creates a store whose value is animated, with a motion that simulates the behavior of a spring. This means when the value changes, instead of transitioning at a steady rate, it "bounces" like a spring would, depending on the physics parameters provided. This adds a level of realism to the transitions and can enhance the user experience. + * The spring function in Svelte creates a store whose value is animated, with a motion that + * simulates the behavior of a spring. This means when the value changes, instead of transitioning + * at a steady rate, it "bounces" like a spring would, depending on the physics parameters provided. + * This adds a level of realism to the transitions and can enhance the user experience. * * https://svelte.dev/docs/svelte-motion#spring * @template [T=any] diff --git a/packages/svelte/src/motion/tweened.js b/packages/svelte/src/motion/tweened.js index 74dca522f24e..b29687871e8b 100644 --- a/packages/svelte/src/motion/tweened.js +++ b/packages/svelte/src/motion/tweened.js @@ -71,7 +71,8 @@ function get_interpolator(a, b) { } /** - * A tweened store in Svelte is a special type of store that provides smooth transitions between state values over time. + * A tweened store in Svelte is a special type of store that provides smooth transitions between + * state values over time. * * https://svelte.dev/docs/svelte-motion#tweened * @template T @@ -89,11 +90,12 @@ export function tweened(value, defaults = {}) { * @param {import('./private').TweenedOptions} [opts] */ function set(new_value, opts) { + target_value = new_value; + if (value == null) { store.set((value = new_value)); return Promise.resolve(); } - target_value = new_value; /** @type {import('../internal/client/private').Task | null} */ let previous_task = task; @@ -123,8 +125,9 @@ export function tweened(value, defaults = {}) { if (now < start) return true; if (!started) { fn = interpolate(/** @type {any} */ (value), new_value); - if (typeof duration === 'function') + if (typeof duration === 'function') { duration = duration(/** @type {any} */ (value), new_value); + } started = true; } if (previous_task) { diff --git a/packages/svelte/tests/motion/test.ts b/packages/svelte/tests/motion/test.ts new file mode 100644 index 000000000000..29a0d23581dd --- /dev/null +++ b/packages/svelte/tests/motion/test.ts @@ -0,0 +1,38 @@ +import { describe, it, assert } from 'vitest'; +import { get } from 'svelte/store'; +import { spring, tweened } from 'svelte/motion'; + +describe('spring', () => { + it('handles initially `undefined` values', () => { + const size = spring(); + + size.set(100); + assert.equal(get(size), 100); + }); +}); + +describe('tweened', () => { + it('handles initially `undefined` values', () => { + const size = tweened(); + + size.set(100); + assert.equal(get(size), 100); + }); + + it('sets immediately when duration is 0', () => { + const size = tweened(0); + + size.set(100, { duration: 0 }); + assert.equal(get(size), 100); + }); + + it('updates correctly when initialized with a `null`-ish value', () => { + const size = tweened(undefined as unknown as number, { duration: 0 }); + + size.set(10); + assert.equal(get(size), 10); + + size.update((v) => v + 10); + assert.equal(get(size), 20); + }); +}); From c084479bc78555025709e60cf4b3a22b147ac6b0 Mon Sep 17 00:00:00 2001 From: mnrx <83848843+mnrx@users.noreply.github.com> Date: Wed, 15 Nov 2023 05:36:39 +0000 Subject: [PATCH 3/5] Add, document, and test new store implementation --- .../docs/03-runtime/02-svelte-store.md | 138 +++- .../svelte/src/internal/client/runtime.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/store/index.js | 626 +++++++++++++----- packages/svelte/src/store/private.d.ts | 36 +- packages/svelte/src/store/public.d.ts | 80 +-- packages/svelte/src/store/utils.js | 31 - packages/svelte/tests/store/test.ts | 312 +++++++-- 8 files changed, 899 insertions(+), 328 deletions(-) delete mode 100644 packages/svelte/src/store/utils.js diff --git a/documentation/docs/03-runtime/02-svelte-store.md b/documentation/docs/03-runtime/02-svelte-store.md index a87a3722ec10..e7301d178835 100644 --- a/documentation/docs/03-runtime/02-svelte-store.md +++ b/documentation/docs/03-runtime/02-svelte-store.md @@ -4,7 +4,7 @@ title: 'svelte/store' The `svelte/store` module exports functions for creating [readable](/docs/svelte-store#readable), [writable](/docs/svelte-store#writable) and [derived](/docs/svelte-store#derived) stores. -Keep in mind that you don't _have_ to use these functions to enjoy the [reactive `$store` syntax](/docs/svelte-components#script-4-prefix-stores-with-$-to-access-their-values) in your components. Any object that correctly implements `.subscribe`, unsubscribe, and (optionally) `.set` is a valid store, and will work both with the special syntax, and with Svelte's built-in [`derived` stores](/docs/svelte-store#derived). +Keep in mind that you don't _have_ to use these functions to enjoy the [reactive `$store` syntax](/docs/svelte-components#script-4-prefix-stores-with-$-to-access-their-values) in your components. Any object that correctly implements the `subscribe` (including returning unsubscribe functions) and (optionally) `set` methods is a valid store, and will work both with the special syntax and with Svelte's built-in [derived stores](/docs/svelte-store#derived). This makes it possible to wrap almost any other reactive state handling library for use in Svelte. Read more about the [store contract](/docs/svelte-components#script-4-prefix-stores-with-$-to-access-their-values) to see what a correct implementation looks like. @@ -18,8 +18,7 @@ Function that creates a store which has values that can be set from 'outside' co `update` is a method that takes one argument which is a callback. The callback takes the existing store value as its argument and returns the new value to be set to the store. -```js -/// file: store.js +```ts import { writable } from 'svelte/store'; const count = writable(0); @@ -35,8 +34,7 @@ count.update((n) => n + 1); // logs '2' If a function is passed as the second argument, it will be called when the number of subscribers goes from zero to one (but not from one to two, etc). That function will be passed a `set` function which changes the value of the store, and an `update` function which works like the `update` method on the store, taking a callback to calculate the store's new value from its old value. It must return a `stop` function that is called when the subscriber count goes from one to zero. -```js -/// file: store.js +```ts import { writable } from 'svelte/store'; const count = writable(0, () => { @@ -59,27 +57,60 @@ Note that the value of a `writable` is lost when it is destroyed, for example wh > EXPORT_SNIPPET: svelte/store#readable -Creates a store whose value cannot be set from 'outside', the first argument is the store's initial value, and the second argument to `readable` is the same as the second argument to `writable`. +Creates a store which can be subscribed to, but does not have the `set` and `update` methods that a writable store has. The value of a readable store is instead set by the `initial_value` argument at creation and then updated internally by an `on_start` function. This function is called when the store receives its first subscriber. + +The `on_start` function allows the creation of stores whose value changes automatically based on application-specific logic. It's passed `set` and `update` functions that behave like the methods available on writable stores. + +The `on_start` function can optionally return an `on_stop` function which will be called when the store loses its last subscriber. This allows stores to go dormant when not being used by any other code. ```ts import { readable } from 'svelte/store'; -const time = readable(new Date(), (set) => { - set(new Date()); +const lastPressedSimpleKey = readable('', (set) => { + const handleEvent = (event: KeyboardEvent) => { + if (event.key.length === 1) { + set(event.key); + } + }; - const interval = setInterval(() => { - set(new Date()); - }, 1000); + window.addEventListener('keypress', handleEvent, { passive: true }); + + return function onStop() { + window.removeEventListener('keypress', handleEvent); + }; +}); - return () => clearInterval(interval); +lastPressedSimpleKey.subscribe((value) => { + console.log(`Most recently pressed simple key is "${value}".`); }); +``` -const ticktock = readable('tick', (set, update) => { +`on_start` could also set up a timer to poll an API for data which changes frequently, or even establish a WebSocket connection. The following example creates a store which polls [Open Notify's public ISS position API](http://open-notify.org/Open-Notify-API/ISS-Location-Now/) every 3 seconds. + +> If `set` or `update` are called after the store has lost its last subscriber, they will have no effect. You should still take care to clean up any asynchronous callbacks registered in `on_start` by providing a suitable `on_stop` function, but a few accidental late calls will not negatively affect the store. +> +> For instance, in the example below `set` may be called late if the `issPosition` store loses its last subscriber after a `fetch` call is made but before the corresponding HTTP response is received. + +```ts +import { readable } from 'svelte/store'; + +const issPosition = readable({ latitude: 0, longitude: 0 }, (set) => { const interval = setInterval(() => { - update((sound) => (sound === 'tick' ? 'tock' : 'tick')); - }, 1000); + fetch('http://api.open-notify.org/iss-now.json') + .then((response) => response.json()) + .then((payload) => set(payload.iss_position)); + }, 3000); + + return function onStop() { + clearInterval(interval); + }; +}); - return () => clearInterval(interval); +issPosition.subscribe(({ latitude, longitude }) => { + console.log( + `The ISS is currently above ${latitude}°, ${longitude}°` + + ` in the ${latitude > 0 ? 'northern' : 'southern'} hemisphere.` + ); }); ``` @@ -108,7 +139,9 @@ import { derived } from 'svelte/store'; const doubled = derived(a, ($a) => $a * 2); ``` -The callback can set a value asynchronously by accepting a second argument, `set`, and an optional third argument, `update`, calling either or both of them when appropriate. +The `derive_value` function can set values asynchronously by accepting a second argument, `set`, and an optional third argument, `update`, and calling either or both of these functions when appropriate. + +> If `set` and `update` are, in combination, called multiple times synchronously, only the last change will cause the store's subscribers to be notified. For instance, calling `update` and then `set` synchronously in a `derive_value` function will only cause the value passed to `set` to be sent to subscribers. In this case, you can also pass a third argument to `derived` — the initial value of the derived store before `set` or `update` is first called. If no initial value is specified, the store's initial value will be `undefined`. @@ -123,7 +156,7 @@ declare global { export {}; // @filename: index.ts -// @errors: 18046 2769 7006 +// @errors: 18046 2769 7006 2722 // ---cut--- import { derived } from 'svelte/store'; @@ -143,7 +176,7 @@ const delayedIncrement = derived(a, ($a, set, update) => { }); ``` -If you return a function from the callback, it will be called when a) the callback runs again, or b) the last subscriber unsubscribes. +If you return a function from the `derive_value` function, it will be called a) before the function runs again, or b) after the last subscriber unsubscribes. ```ts // @filename: ambient.d.ts @@ -188,7 +221,6 @@ declare global { export {}; // @filename: index.ts - // ---cut--- import { derived } from 'svelte/store'; @@ -199,13 +231,75 @@ const delayed = derived([a, b], ([$a, $b], set) => { }); ``` +### TypeScript type inference + +If a multi-argument `derive_value` function is passed to `derive`, TypeScript may not be able to infer the type of the derived store, yielding a store of type `Readable`. Set an initial value for the store to resolve this; `undefined` with a type assertion is sufficient. Alternatively, you may use type arguments, although this requires specifying the types of the dependency stores, too. + +```ts +// @filename: ambient.d.ts +import { type Writable } from 'svelte/store'; + +declare global { + const a: Writable; +} + +export {}; + +// @filename: index.ts +// ---cut--- +import { derived } from 'svelte/store'; + +// @errors: 2769 +const aInc = derived( + a, + ($a, set) => setTimeout(() => set($a + 1), 1000), + undefined as unknown as number +); + +const concatenated = derived<[number, number], string>([a, aInc], ([$a, $aInc], set) => + setTimeout(() => set(`${$a}${$aInc}`), 1000) +); +``` + +`derived` can derive new stores from stores not created by Svelte, including from RxJS `Observable`s. In this case, TypeScript may not be able to infer the types of data held by the dependency stores. Use a type assertion to `ExternalStore` or a type argument to provide the missing context. + +Until TypeScript gains support for [partial type argument inference](https://github.com/microsoft/TypeScript/issues/26242), the latter option requires also specifying the return type of `derive_store`. + +```ts +// @filename: ambient.d.ts +import { type Writable } from 'svelte/store'; + +declare global { + const a: Writable; + const observable: { + subscribe: (fn: (value: unknown) => void) => { unsubscribe: () => void }; + }; +} + +export {}; + +// @filename: index.ts +// ---cut--- +import { derived, type ExternalStore } from 'svelte/store'; + +const sum = derived( + [a, observable as ExternalStore], + ([$a, $observable]) => $a + $observable +); + +const sumMore = derived<[number, number], number>( + [sum, observable], + ([$sum, $observable]) => $sum + $observable +); +``` + ## `readonly` > EXPORT_SNIPPET: svelte/store#readonly This simple helper function makes a store readonly. You can still subscribe to the changes from the original one using this new readable store. -```js +```ts import { readonly, writable } from 'svelte/store'; const writableStore = writable(1); @@ -224,7 +318,7 @@ readableStore.set(2); // ERROR Generally, you should read the value of a store by subscribing to it and using the value as it changes over time. Occasionally, you may need to retrieve the value of a store to which you're not subscribed. `get` allows you to do so. -> This works by creating a subscription, reading the value, then unsubscribing. It's therefore not recommended in hot code paths. +> By default, `get` subscribes to the given store, makes note of its value, then unsubscribes again. Passing `true` as a second argument causes `get` to directly read the internal state of the store instead, which, in the case of a derived store, may be outdated or `undefined`. Where performance is important, it's recommended to set `allow_stale` to `true` or not use `get`. ```ts // @filename: ambient.d.ts diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a67f300b219a..f825425c2128 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1,4 +1,4 @@ -import { subscribe_to_store } from '../../store/utils.js'; +import { subscribe_to_store } from '../../store/index.js'; import { EMPTY_FUNC } from '../common.js'; import { unwrap } from './render.js'; import { map_delete, map_get, map_set } from './operations.js'; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 080cc13d72fb..d6841505289b 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -1,7 +1,7 @@ import * as $ from '../client/runtime.js'; import { set_is_ssr } from '../client/runtime.js'; import { is_promise } from '../common.js'; -import { subscribe_to_store } from '../../store/utils.js'; +import { subscribe_to_store } from '../../store/index.js'; export * from '../client/validate.js'; diff --git a/packages/svelte/src/store/index.js b/packages/svelte/src/store/index.js index 857eab0af8e1..93e4c905de77 100644 --- a/packages/svelte/src/store/index.js +++ b/packages/svelte/src/store/index.js @@ -1,30 +1,51 @@ -import { subscribe_to_store } from './utils.js'; +import { EMPTY_FUNC } from '../internal/common.js'; + +const value_key = Symbol('value'); +const is_derived_key = Symbol('is_derived'); +const is_syncing_key = Symbol('is_syncing'); +const subscribers_key = Symbol('subscribers'); +const notify_key = Symbol('notify'); +const set_key = Symbol('set'); +const update_key = Symbol('update'); /** - * @type {Array | any>} + * Extended `symbol` properties of `Store` that cannot be defined outside of + * this file without exporting the `symbol`s. + * + * @template T + * @typedef {{ + * [value_key]: T, + * [is_derived_key]?: boolean, + * [is_syncing_key]?: boolean, + * [subscribers_key]: import('./private.js').Subscribers, + * [notify_key]: (is_changed: boolean) => void, + * [set_key]: import('./public.js').Setter, + * [update_key]: (fn: import('./public.js').Updater) => void + * }} StoreSymbols */ -const subscriber_queue = []; - -/** @returns {void} */ -// eslint-disable-next-line @typescript-eslint/no-empty-function -function noop() {} /** - * Creates a `Readable` store that allows reading by subscription. + * `Readable` with extended `symbol` properties. * - * https://svelte.dev/docs/svelte-store#readable * @template T - * @param {T} [value] initial value - * @param {import('./public').StartStopNotifier} [start] - * @returns {import('./public').Readable} + * @typedef {import('./public.js').Readable & StoreSymbols} Readable + */ +/** + * `Writable` with extended `symbol` properties. + * + * @template T + * @typedef {import('./public.js').Writable & StoreSymbols} Writable */ -export function readable(value, start) { - return { - subscribe: writable(value, start).subscribe - }; -} + +/** @returns {void} */ +// eslint-disable-next-line @typescript-eslint/no-empty-function +function noop() {} /** + * Return `true` if `a` and `b` are unequal. Unlike usual, identical `Function`s + * and non-`null` `Object`s are considered to be unequal to themselves. + * Also unlike usual, `NaN` *is* considered to be equal to itself. + * * @param {any} a * @param {any} b * @returns {boolean} @@ -38,180 +59,475 @@ export function safe_not_equal(a, b) { } /** - * Create a `Writable` store that allows both updating and reading by subscription. + * Create a `DisableableSetterUpdater` for the given store. + * + * @template T + * @param {Readable} store + * @returns {import('./private.js').DisableableSetterUpdater} + */ +const create_disableable_setter_updater = (store) => { + /** @type {import('./private.js').DisableableSetterUpdater} */ + const state = { + enabled: true, + set(value) { + if (this.enabled) store[set_key](value); + }, + update(fn) { + if (this.enabled) store[update_key](fn); + } + }; + state.set = state.set.bind(state); + state.update = state.update.bind(state); + return state; +}; + +/** + * Create an internal store. * - * https://svelte.dev/docs/svelte-store#writable * @template T - * @param {T} [value] initial value - * @param {import('./public').StartStopNotifier} [start] - * @returns {import('./public').Writable} - */ -export function writable(value, start = noop) { - /** @type {import('./public').Unsubscriber | null} */ - let stop = null; - - /** @type {Set>} */ - const subscribers = new Set(); - - /** - * @param {T} new_value - * @returns {void} - */ - function set(new_value) { - if (safe_not_equal(value, new_value)) { - value = new_value; - if (stop) { - // store is ready - const run_queue = !subscriber_queue.length; + * @param {T} [initial_value] Initial value + * @param {( + * import('./public.js').OnStart + * | import('./private.js').DerivedOnStart + * )} [on_start = noop] + * A function called when the store receives its first subscriber. + * @returns {Readable} + */ +const create_store = (initial_value, on_start = noop) => { + /** @type {import('./private.js').Subscribers} */ + const subscribers = []; + let on_stop = noop; + + /** @type {Readable} */ + const store = { + [value_key]: /** @type {T} */ (initial_value), + [subscribers_key]: subscribers, + + [notify_key](is_changed) { + for (const subscriber of subscribers) { + if (Array.isArray(subscriber)) { + if (is_changed || this[is_derived_key]) { + subscriber[0](this[value_key], is_changed); + } + } else { + if (is_changed) subscriber(this[value_key]); + } + } + }, + + [set_key](value) { + const is_changed = safe_not_equal(value, this[value_key]); + + if (is_changed && !this[is_syncing_key]) { for (const subscriber of subscribers) { - subscriber[1](); - subscriber_queue.push(subscriber, value); + if (Array.isArray(subscriber)) subscriber[1](); } - if (run_queue) { - for (let i = 0; i < subscriber_queue.length; i += 2) { - subscriber_queue[i][0](subscriber_queue[i + 1]); - } - subscriber_queue.length = 0; + } + + if (is_changed) this[value_key] = value; + + if (!this[is_syncing_key]) this[notify_key](is_changed); + }, + + [update_key](fn) { + this[set_key](fn(this[value_key])); + }, + + subscribe(subscriber) { + /** @type {import('./private.js').DisableableSetterUpdater} */ + let setter_updater; + + if (subscribers.length === 0) { + if (this[is_derived_key]) { + // @ts-ignore + on_stop = on_start(this) || noop; + } else { + setter_updater = create_disableable_setter_updater(this); + // @ts-ignore + on_stop = on_start(setter_updater.set, setter_updater.update) || noop; } } - } - } - /** - * @param {import('./public').Updater} fn - * @returns {void} - */ - function update(fn) { - set(fn(/** @type {T} */ (value))); - } + subscribers.push(subscriber); + (Array.isArray(subscriber) ? subscriber[0] : subscriber)(this[value_key]); + + return function unsubscribe() { + const subscriber_index = subscribers.indexOf(subscriber); + if (subscriber_index !== -1) subscribers.splice(subscriber_index, 1); - /** - * @param {import('./public').Subscriber} run - * @param {import('./private').Invalidator} [invalidate] - * @returns {import('./public').Unsubscriber} - */ - function subscribe(run, invalidate = noop) { - /** @type {import('./private').SubscribeInvalidateTuple} */ - const subscriber = [run, invalidate]; - subscribers.add(subscriber); - if (subscribers.size === 1) { - stop = start(set, update) || noop; + if (subscribers.length === 0) { + on_stop(); + if (setter_updater !== undefined) setter_updater.enabled = false; + } + }; } - run(/** @type {T} */ (value)); - return () => { - subscribers.delete(subscriber); - if (subscribers.size === 0 && stop) { - stop(); - stop = null; - } - }; - } - return { set, update, subscribe }; + }; + + // Allow consumers to pass around a store's `subscribe` method (e.g., to + // another function) more liberally without breaking it. + store.subscribe = store.subscribe.bind(store); + + return store; +}; + +/** + * Create a `Writable` store that allows both updating and reading by + * subscription. + * + * https://svelte.dev/docs/svelte-store#writable + * + * @template T + * @param {T} [initial_value] Initial value + * @param {import('./public.js').OnStart} [on_start = noop] + * A function called when the store receives its first subscriber. + * @returns {Writable} + */ +export function writable(initial_value, on_start = noop) { + const store = /** @type {Writable} */ (create_store(initial_value, on_start)); + store.update = store[update_key].bind(store); + store.set = store[set_key].bind(store); + return store; } -/** @param {Function} fn */ -function run(fn) { - return fn(); +/** + * Creates a `Readable` store that allows reading by subscription. + * + * https://svelte.dev/docs/svelte-store#readable + * + * @template T + * @param {T} [initial_value] Initial value + * @param {import('./public.js').OnStart} [on_start] + * A function called when the store receives its first subscriber. + * @returns {Readable} + */ +export function readable(initial_value, on_start) { + return create_store(initial_value, on_start); } +/** @type {Map, Readable>} */ +const wrapped_stores = new Map(); +/** + * Wrap an external (non-native) store as a `Readable`. Stores implementing + * RxJS's `Observable` interface are accepted. + * + * @template {Readable} S + * @overload + * @param {S} store Store to wrap. + * @returns {S} + */ /** - * @param {Function[]} fns - * @returns {void} + * Wrap an external (non-native) store as a `Readable`. Stores implementing + * RxJS's `Observable` interface are accepted. + * + * @template {import('./public.js').ExternalReadable} S + * @overload + * @param {S} store Store to wrap. + * @returns {S extends import('./public.js').ExternalReadable ? Readable : never} + */ +/** + * Wrap an external (non-native) store as a `Readable`. Stores implementing + * RxJS's `Observable` interface are accepted. + * + * @template [T = unknown] + * @overload + * @param {import('./public.js').ExternalReadable} store Store to wrap. + * @returns {Readable} */ -function run_all(fns) { - fns.forEach(run); +function wrap_store( + /** @type {import('./public.js').ExternalReadable | Readable} */ + store +) { + if (store.hasOwnProperty(set_key)) { + return /** @type {Readable} */ (store); + } + + if (wrapped_stores.has(store)) { + return /** @type {Readable} */ (wrapped_stores.get(store)); + } + + const wrapped_store = readable( + undefined, + /** @type {import('./public.js').OnStart} */ + (set) => { + const unsubscribe = store.subscribe(set); + return function on_stop() { + const is_rxjs = typeof unsubscribe !== 'function'; + (is_rxjs ? unsubscribe.unsubscribe : unsubscribe)(); + }; + } + ); + wrapped_stores.set(store, wrapped_store); + return wrapped_store; } /** - * @template {import('./private').Stores} S - * @template T - * @param {S} stores - * @param {Function} fn - * @param {T} [initial_value] - * @returns {import('./public').Readable} - */ -export function derived(stores, fn, initial_value) { - const single = !Array.isArray(stores); - /** @type {Array>} */ - const stores_array = single ? [stores] : stores; - if (!stores_array.every(Boolean)) { - throw new Error('derived() expects stores as input, got a falsy value'); + * Create a new `Readable` store whose value is derived from the value(s) of one + * or more other `Readable` stores and whose value is re-evaluated whenever one + * or more dependency store updates. + * + * https://svelte.dev/docs/svelte-store#derived + * + * @template {import('./private.js').Stores} S + * @template [T = unknown] + * @overload + * @param {S} dependency_or_dependencies Dependency store or array of stores. + * @param {import('./public.js').ComplexDeriveValue} derive_value + * Function which derives a value from the dependency stores' values and + * optionally calls the passed `set` or `update` functions to change the + * store. + * @param {T} [initial_value] Initial value. + * @returns {Readable} + */ +/** + * Create a new `Readable` store whose value is derived from the value(s) of one + * or more other `Readable` stores and whose value is re-evaluated whenever one + * or more dependency store updates. + * + * https://svelte.dev/docs/svelte-store#derived + * + * @template {import('./private.js').Stores} S + * @template [T = unknown] + * @overload + * @param {S} dependency_or_dependencies Dependency store or array of stores. + * @param {import('./public.js').SimpleDeriveValue} derive_value + * Function which derives and returns a value from the dependency stores' + * values. + * @param {T} [initial_value] Initial value. + * @returns {Readable} + */ +/** + * Create a new `Readable` store whose value is derived from the value(s) of one + * or more other `Readable` stores and whose value is re-evaluated whenever one + * or more dependency store updates. + * + * https://svelte.dev/docs/svelte-store#derived + * + * @template Ts + * @template [T = unknown] + * @overload + * @param {import('./private.js').Stores} dependency_or_dependencies Dependency + * store or array of stores. + * @param {import('./public.js').ComplexDeriveValue} derive_value + * Function which derives a value from the dependency stores' values and + * optionally calls the passed `set` or `update` functions to change the store. + * @param {T} [initial_value] Initial value. + * @returns {Readable} + */ +/** + * Create a new `Readable` store whose value is derived from the value(s) of one + * or more other `Readable` stores and whose value is re-evaluated whenever one + * or more dependency store updates. + * + * https://svelte.dev/docs/svelte-store#derived + * + * @template Ts + * @template [T = unknown] + * @overload + * @param {import('./private.js').Stores} dependency_or_dependencies Dependency + * store or array of stores. + * @param {import('./public.js').SimpleDeriveValue} derive_value + * Function which derives and returns a value from the dependency stores' + * values. + * @param {T} [initial_value] Initial value. + * @returns {Readable} + */ +export function derived( + /** @type {S} */ + dependency_or_dependencies, + /** @type {import('./public.js').ComplexDeriveValue | import('./public.js').SimpleDeriveValue} */ + derive_value, + /** @type {T} */ + initial_value +) { + const has_single_dependency = !Array.isArray(dependency_or_dependencies); + /** @type {import('./private.js').Stores} */ + const dependencies = has_single_dependency + ? [dependency_or_dependencies] + : dependency_or_dependencies; + + for (const [i, dependency] of dependencies.entries()) { + if (!dependency) { + throw Error(`Dependency with index ${i} passed to \`derived()\` is falsy.`); + } } - const auto = fn.length < 2; - return readable(initial_value, (set, update) => { - let started = false; - /** @type {T[]} */ - const values = []; - let pending = 0; - let cleanup = noop; - const sync = () => { - if (pending) { - return; - } - cleanup(); - const result = fn(single ? values[0] : values, set, update); - if (auto) { - set(result); - } else { - cleanup = typeof result === 'function' ? result : noop; - } - }; - const unsubscribers = stores_array.map((store, i) => - subscribe_to_store( - store, - (value) => { - values[i] = value; - pending &= ~(1 << i); - if (started) { - sync(); - } - }, - () => { - pending |= 1 << i; + + /** @type {Array>} */ + const wrapped_dependencies = dependencies.map(wrap_store); + + const store = create_store( + initial_value, + // @ ts-expect-error Required to keep `StoreSymbols` out of the public + // type definitions. + /** @type {import('./private.js').DerivedOnStart} */ + ( + function on_start( + /** @type {Writable} */ + store + ) { + /** @type {Array<() => void>} */ + const unsubscribers = []; + let pending_store_count = wrapped_dependencies.length; + let is_invalid = false; + + let clean_up = noop; + for (const dependency of wrapped_dependencies) { + /** @type {import('./private.js').DisableableSetterUpdater} */ + let setter_updater; + + const unsubscribe = dependency.subscribe([ + function on_value_change(_, is_changed = true) { + if (is_changed) is_invalid = true; + pending_store_count -= 1; + + if (pending_store_count === 0) { + const old_value = store[value_key]; + + if (is_invalid) { + clean_up(); + clean_up = noop; + is_invalid = false; + store[is_syncing_key] = true; + const store_values = wrapped_dependencies.map( + (dependency) => dependency[value_key] + ); + const store_values_arg = has_single_dependency ? store_values[0] : store_values; + + if (derive_value.length === 1) { + // @ts-expect-error TypeScript does not narrow types on `Function.length`, and so + // cannot differentiate `SimpleDeriveValue` and `ComplexDeriveValue`. + store[set_key](derive_value(store_values_arg)); + } else { + setter_updater = create_disableable_setter_updater(store); + const derived_value = derive_value( + store_values_arg, + setter_updater.set, + setter_updater.update + ); + clean_up = + typeof derived_value === 'function' + ? /** @type {typeof noop} */ (derived_value) + : noop; + } + store[is_syncing_key] = false; + } + store[notify_key](safe_not_equal(store[value_key], old_value)); + } + }, + function invalidate() { + if (pending_store_count === 0) { + for (const subscriber of store[subscribers_key]) { + if (Array.isArray(subscriber)) subscriber[1](); + } + } + pending_store_count += 1; + } + ]); + + unsubscribers.push(() => { + clean_up(); + if (setter_updater !== undefined) setter_updater.enabled = false; + unsubscribe(); + }); } - ) - ); - started = true; - sync(); - return function stop() { - run_all(unsubscribers); - cleanup(); - // We need to set this to false because callbacks can still happen despite having unsubscribed: - // Callbacks might already be placed in the queue which doesn't know it should no longer - // invoke this derived store. - started = false; - }; - }); + + return function on_stop() { + for (const unsubscribe of unsubscribers) unsubscribe(); + }; + } + ) + ); + + store[is_derived_key] = true; + + return store; } /** - * Takes a store and returns a new one derived from the old one that is readable. + * Return a `Readable` of an existing `Readable` or `Writable` store. * * https://svelte.dev/docs/svelte-store#readonly + * * @template T - * @param {import('./public').Readable} store - store to make readonly - * @returns {import('./public').Readable} + * @param {Readable} store Store to make read-only. + * @returns {Readable} */ -export function readonly(store) { - return { - // @ts-expect-error TODO i suspect the bind is unnecessary +export function readonly(/** @type {Readable} */ store) { + /** @type {Readable} */ + const read_only_store = { + get [value_key]() { + return store[value_key]; + }, + [subscribers_key]: store[subscribers_key], + [notify_key]: store[notify_key].bind(store), + [set_key]: store[set_key].bind(store), + [update_key]: store[update_key].bind(store), subscribe: store.subscribe.bind(store) }; + return read_only_store; } /** - * Get the current value from a store by subscribing and immediately unsubscribing. + * Get the current value from a store by subscribing and immediately + * unsubscribing. If `allow_stale` is `true`, the current value is read directly + * from compatible store objects. This is faster but potentially inaccurate. + * + * @template T + * @overload + * @param {Readable} store Store to get value of. + * @param {boolean} [allow_stale = false] Allow reading potentially stale values + * from the store rather than subscribing and unsubscribing. + * @returns {T} + */ +/** + * Get the current value from a store by subscribing and immediately + * unsubscribing. If `allow_stale` is `true`, the current value is read directly + * from compatible store objects. This is faster but potentially inaccurate. * - * https://svelte.dev/docs/svelte-store#get * @template T - * @param {import('../store/public').Readable} store + * @overload + * @param {Readable | import('./public.js').ExternalReadable} store + * Store to get value of. * @returns {T} */ -export function get_store_value(store) { - let value; - subscribe_to_store(store, (_) => (value = _))(); - // @ts-expect-error - return value; +export function get_store_value(store, allow_stale = false) { + if (allow_stale && store.hasOwnProperty(set_key)) { + // @ts-expect-error TypeScript does not narrow types on `Object.hasOwnProperty`, and so cannot + // differentiate `Readable` and `ExternalReadable`. + return store[value_key]; + } + + /** @type {T} */ + let fresh_value; + const unsubscribe = store.subscribe((value) => (fresh_value = value)); + const is_rxjs = typeof unsubscribe !== 'function'; + (is_rxjs ? unsubscribe.unsubscribe : unsubscribe)(); + + // @ts-expect-error `T | undefined` is more accurate but causes more headache. + return fresh_value; } export { get_store_value as get }; + +/** + * @template T + * @param {import('./public.js').Readable | import('./public.js').ExternalReadable | null | undefined} store + * @param {(value: T) => void} run + * @param {(value: T) => void} [invalidate] + * @returns {() => void} + */ +export function subscribe_to_store(store, run, invalidate) { + if (store == null) { + // @ts-expect-error + run(undefined); + + // @ts-expect-error + if (invalidate) invalidate(undefined); + + return EMPTY_FUNC; + } + + // @ts-expect-error `SubscriberInvalidator` is not public. + const unsubscribe = store.subscribe([run, invalidate]); + const is_rxjs = typeof unsubscribe !== 'function'; + return is_rxjs ? unsubscribe.unsubscribe : unsubscribe; +} diff --git a/packages/svelte/src/store/private.d.ts b/packages/svelte/src/store/private.d.ts index 3fe08f832813..adbb3f50f70b 100644 --- a/packages/svelte/src/store/private.d.ts +++ b/packages/svelte/src/store/private.d.ts @@ -1,18 +1,34 @@ -import { Readable, Subscriber } from './public.js'; +import { Updater, Subscriber, ExternalReadable, Readable, Writable } from './public.js'; -/** Cleanup logic callback. */ -export type Invalidator = (value?: T) => void; +/** An object with `set` and `update` methods that only work if the object's `enabled` property is + * `true` */ +export type DisableableSetterUpdater = { + enabled: boolean; + set: (value: T) => void; + update: (fn: Updater) => void; +}; -/** Pair of subscriber and invalidator. */ -export type SubscribeInvalidateTuple = [Subscriber, Invalidator]; +/** A tuple of a function which is called when a store's value changes and a function which is + * called shortly before the first to enable proper order of evaluation. */ +export type SubscriberInvalidator = [(value: T, isChanged?: boolean) => void, () => void]; -/** One or more `Readable`s. */ +/** An `Array` of all subscribers to a store. */ +export type Subscribers = Array | SubscriberInvalidator>; + +/** One or more `Readable` or `ExternalReadable` stores. The spread syntax is important for + * `StoresValues` to work. */ export type Stores = | Readable - | [Readable, ...Array>] - | Array>; + | ExternalReadable + | [Readable | ExternalReadable, ...Array | ExternalReadable>]; /** One or more values from `Readable` stores. */ -export type StoresValues = T extends Readable +export type StoresValues = T extends Readable | ExternalReadable ? U - : { [K in keyof T]: T[K] extends Readable ? U : never }; + : { + [K in keyof T]: T[K] extends Readable | ExternalReadable ? U : never; + }; + +/** A special case of `OnStart` created by `derived` when subscribing to other stores created by + * this module. */ +export type DerivedOnStart = (store: Writable) => () => void; diff --git a/packages/svelte/src/store/public.d.ts b/packages/svelte/src/store/public.d.ts index 26f887e0e010..88c28d14d542 100644 --- a/packages/svelte/src/store/public.d.ts +++ b/packages/svelte/src/store/public.d.ts @@ -1,51 +1,43 @@ -import type { Invalidator } from './private.js'; +import { SubscriberInvalidator, Stores, StoresValues } from './private.js'; -/** Callback to inform of a value updates. */ -export type Subscriber = (value: T) => void; - -/** Unsubscribes from value updates. */ -export type Unsubscriber = () => void; +/** A function which sets a store's value. */ +export type Setter = (value: T) => void; -/** Callback to update a value. */ +/** A function which returns a new value for a store derived from its current value. */ export type Updater = (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: Updater) => 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 = ( - set: (value: T) => void, +/** A function which is called when a store's value changes. */ +export type Subscriber = (value: T) => void; + +/** A store not created by this module, e.g., an RxJS `Observable`. */ +export type ExternalReadable = { + subscribe: (subscriber: Subscriber) => (() => void) | { unsubscribe: () => void }; +}; + +/** A store created by this module which can be subscribed to. */ +export type Readable = { + subscribe: (subscriber: Subscriber | SubscriberInvalidator) => () => void; +}; + +/** A store which can be subscribed to and has `set` and `update` methods. */ +export type Writable = Readable & { + set: Setter; + update: (fn: Updater) => void; +}; + +/** A function which is called whenever a store receives its first subscriber, and whose return + * value, if a function, is called when the same store loses its last subscriber. */ +export type OnStart = (set: Setter, update: (fn: Updater) => void) => void | (() => void); + +/** A function which derives a value from the dependency stores' values and optionally calls the + * passed `set` or `update` functions to change the store. */ +export type ComplexDeriveValue = ( + values: S extends Stores ? StoresValues : S, + set: Setter, update: (fn: Updater) => void ) => void | (() => 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): Unsubscriber; -} - -/** Writable interface for both updating and subscribing. */ -export interface Writable extends Readable { - /** - * Set value and inform subscribers. - * @param value to set - */ - set(this: void, value: T): void; - - /** - * Update value using callback and inform subscribers. - * @param updater callback - */ - update(this: void, updater: Updater): void; -} - -export * from './index.js'; +/** A function which derives a value from the dependency stores' values and returns it. */ +export type SimpleDeriveValue = (values: S extends Stores ? StoresValues : S) => T; + +export { writable, readable, derived, readonly, get } from './index.js'; diff --git a/packages/svelte/src/store/utils.js b/packages/svelte/src/store/utils.js deleted file mode 100644 index 979a2518d8f0..000000000000 --- a/packages/svelte/src/store/utils.js +++ /dev/null @@ -1,31 +0,0 @@ -import { EMPTY_FUNC } from '../internal/common.js'; - -/** - * @template T - * @param {import('./public').Readable | null | undefined} store - * @param {(value: T) => void} run - * @param {(value: T) => void} [invalidate] - * @returns {() => void} - */ -export function subscribe_to_store(store, run, invalidate) { - if (store == null) { - // @ts-expect-error - run(undefined); - - // @ts-expect-error - if (invalidate) invalidate(undefined); - - return EMPTY_FUNC; - } - - // Svelte store takes a private second argument - const unsub = store.subscribe( - run, - // @ts-expect-error - invalidate - ); - - // Also support RxJS - // @ts-expect-error TODO fix this in the types? - return unsub.unsubscribe ? () => unsub.unsubscribe() : unsub; -} diff --git a/packages/svelte/tests/store/test.ts b/packages/svelte/tests/store/test.ts index c60b9c687ea8..09af16f25b8d 100644 --- a/packages/svelte/tests/store/test.ts +++ b/packages/svelte/tests/store/test.ts @@ -103,6 +103,23 @@ describe('writable', () => { unsubscribe(); assert.doesNotThrow(() => unsubscribe()); }); + + it('allows multiple subscriptions of one handler', () => { + let call_count = 0; + + const value = writable(1); + + const handle_new_value = () => { + call_count += 1; + }; + const unsubscribers = [1, 2, 3].map((_) => value.subscribe(handle_new_value)); + + assert.equal(call_count, 3); + value.set(2); + assert.equal(call_count, 6); + + for (const unsubscribe of unsubscribers) unsubscribe(); + }); }); describe('readable', () => { @@ -234,8 +251,6 @@ const fake_observable = { describe('derived', () => { it('maps a single store', () => { const a = writable(1); - - // @ts-expect-error TODO feels like inference should work here const b = derived(a, (n) => n * 2); const values: number[] = []; @@ -256,8 +271,6 @@ describe('derived', () => { it('maps multiple stores', () => { const a = writable(2); const b = writable(3); - - // @ts-expect-error TODO feels like inference should work here const c = derived([a, b], ([a, b]) => a * b); const values: number[] = []; @@ -276,11 +289,46 @@ describe('derived', () => { assert.deepEqual(values, [6, 12, 20]); }); + it('allows derived with different types', () => { + const a = writable('one'); + const b = writable(1); + const c = derived([a, b], ([a, b]) => `${a} ${b}`); + + assert.deepEqual(get(c), 'one 1'); + + a.set('two'); + b.set(2); + assert.deepEqual(get(c), 'two 2'); + }); + + it('errors on undefined stores #1', () => { + assert.throws(() => { + // @ts-expect-error `null` and `undefined` should create type errors, but this code is testing + // that the implementation also throws an error. + derived(null, (n) => n); + }); + }); + + it('errors on undefined stores #2', () => { + assert.throws(() => { + const a = writable(1); + // @ts-expect-error `null` and `undefined` should create type errors, but this code is testing + // that the implementation also throws an error. + derived([a, null, undefined], ([n]) => { + return n * 2; + }); + }); + }); + + it('works with RxJS-style observables', () => { + const d = derived(fake_observable, (_) => _); + assert.equal(get(d), 42); + }); + it('passes optional `set` function', () => { const number = writable(1); const evens = derived( number, - // @ts-expect-error TODO feels like inference should work here (n, set) => { if (n % 2 === 0) set(n); }, @@ -311,10 +359,8 @@ describe('derived', () => { const number = writable(1); const evens_and_squares_of_4 = derived( number, - // @ts-expect-error TODO feels like inference should work here (n, set, update) => { if (n % 2 === 0) set(n); - // @ts-expect-error TODO feels like inference should work here if (n % 4 === 0) update((n) => n * n); }, 0 @@ -331,28 +377,24 @@ describe('derived', () => { number.set(4); number.set(5); number.set(6); - assert.deepEqual(values, [0, 2, 4, 16, 6]); + assert.deepEqual(values, [0, 2, 16, 6]); number.set(7); number.set(8); number.set(9); number.set(10); - assert.deepEqual(values, [0, 2, 4, 16, 6, 8, 64, 10]); + assert.deepEqual(values, [0, 2, 16, 6, 64, 10]); unsubscribe(); number.set(11); number.set(12); - assert.deepEqual(values, [0, 2, 4, 16, 6, 8, 64, 10]); + assert.deepEqual(values, [0, 2, 16, 6, 64, 10]); }); it('prevents glitches', () => { const last_name = writable('Jekyll'); - - // @ts-expect-error TODO feels like inference should work here const first_name = derived(last_name, (n) => (n === 'Jekyll' ? 'Henry' : 'Edward')); - - // @ts-expect-error TODO feels like inference should work here const full_name = derived([first_name, last_name], (names) => names.join(' ')); const values: string[] = []; @@ -368,21 +410,39 @@ describe('derived', () => { unsubscribe(); }); + it('only calls `derive_value` when necessary', () => { + let call_count = 0; + + const sequence = writable(['a', 'b']); + const length = derived(sequence, ($sequence) => { + call_count += 1; + return $sequence.length; + }); + + assert.equal(call_count, 0); + + const unsubscribes = [ + length.subscribe(() => {}), + length.subscribe(() => {}), + length.subscribe(() => {}) + ]; + assert.equal(call_count, 1); + + for (const unsubscribe of unsubscribes) unsubscribe(); + }); + it('prevents diamond dependency problem', () => { const count = writable(0); const values: string[] = []; - // @ts-expect-error TODO feels like inference should work here const a = derived(count, ($count) => { return 'a' + $count; }); - // @ts-expect-error TODO feels like inference should work here const b = derived(count, ($count) => { return 'b' + $count; }); - // @ts-expect-error TODO feels like inference should work here const combined = derived([a, b], ([a, b]) => { return a + b; }); @@ -399,17 +459,178 @@ describe('derived', () => { unsubscribe(); }); + it('avoids premature updates of dependent stores on invalid state', () => { + const values: number[] = []; + + const sequence = writable(['a', 'b']); + const length = derived(sequence, ($sequence) => $sequence.length); + const lengths_sum = derived( + [sequence, length], + ([$sequence, $length]) => $sequence.length + $length + ); + + const unsubscribe = lengths_sum.subscribe((value) => values.push(value)); + + assert.deepEqual(values, [4]); + sequence.set(['a', 'b', 'c']); + assert.deepEqual(values, [4, 6]); + + unsubscribe(); + }); + + it('avoids premature updates of dependent stores on invalid state', () => { + const values: number[] = []; + + const sequence = writable(['a', 'b']); + const length = derived(sequence, ($sequence) => $sequence.length); + const length_dec = derived(length, ($length) => $length - 1); + const lengths_sum = derived( + [sequence, length_dec], + ([$sequence, $length_dec]) => $sequence.length + $length_dec + ); + + const unsubscribe = lengths_sum.subscribe((value) => values.push(value)); + + assert.deepEqual(values, [3]); + sequence.set(['a', 'b', 'c']); + assert.deepEqual(values, [3, 5]); + + unsubscribe(); + }); + + it('avoids premature updates of dependent stores on invalid state', () => { + const values: string[] = []; + + const length = writable(2); + const length_dec = derived(length, ($length) => $length - 1); + const length_dec_inc = derived(length_dec, ($length_dec) => $length_dec + 1); + const sequence = derived(length_dec_inc, ($length_dec_inc) => + [...Array($length_dec_inc)].map((_, i) => i) + ); + const last_as_string = derived([length_dec_inc, sequence], ([$length_dec_inc, $sequence]) => + $sequence[$length_dec_inc - 1].toString() + ); + + const unsubscribe = last_as_string.subscribe((value) => values.push(value)); + + assert.deepEqual(values, ['1']); + length.set(3); + assert.deepEqual(values, ['1', '2']); + + unsubscribe(); + }); + + it('does not freeze when depending on an asynchronous store', () => { + const values: number[] = []; + + const noop = () => {}; + // `do_later` allows deferring store updates in `length_delayed` without having to handle + // strictly asynchronous execution. + let do_later = noop; + + const length = writable(1); + const length_delayed = derived( + length, + ($length, set) => { + do_later = () => { + set($length); + }; + return () => { + do_later = noop; + }; + }, + 0 + ); + const lengths_derivative = derived( + [length, length_delayed], + ([$length, $length_delayed]) => $length * 3 - $length_delayed + ); + + const unsubscribe = lengths_derivative.subscribe((value) => values.push(value)); + + if (typeof do_later === 'function') do_later(); + length.set(2); + length.set(3); + length.set(4); + if (typeof do_later === 'function') do_later(); + if (typeof do_later === 'function') do_later(); + assert.deepEqual(values, [3, 2, 5, 8, 11, 8]); + + unsubscribe(); + }); + + it('disables `set` and `update` functions during `on_start` clean-up (`on_stop`)', () => { + const noop = () => {}; + // `do_later()` allows deferring store updates in `length_delayed` without having to handle + // strictly asynchronous execution. + let do_later = noop; + + const length = readable(0, (set) => { + if (do_later === noop) do_later = () => set(1); + // No clean-up function is returned, so `do_later()` remains set even after it should. + }); + + assert.equal(get(length, true), 0); + + const unsubscribe = length.subscribe(noop); + unsubscribe(); + + assert.equal(get(length, true), 0); + if (typeof do_later === 'function') do_later(); + assert.equal(get(length, true), 0); + + // The original `set()` is still disabled even after re-subscribing, since `set` and `update` + // are created anew each time. + const unsubscribe_2 = length.subscribe(noop); + if (typeof do_later === 'function') do_later(); + assert.equal(get(length, true), 0); + + unsubscribe_2(); + }); + + it('disables `set` and `update` functions during `derived` clean-up', () => { + const noop = () => {}; + // `do_later()` allows deferring store updates in `length_delayed` without having to handle + // strictly asynchronous execution. + let do_later = noop; + + const length = writable(1); + const length_delayed = derived( + length, + ($length, _, update) => { + if (do_later === noop) do_later = () => update((_) => $length); + // No clean-up function is returned, so `do_later()` remains set even after it should. + }, + 0 + ); + + assert.equal(get(length_delayed, true), 0); + + const unsubscribe = length_delayed.subscribe(noop); + unsubscribe(); + + assert.equal(get(length_delayed, true), 0); + if (typeof do_later === 'function') do_later(); + assert.equal(get(length_delayed, true), 0); + + // The original `update()` is still disabled even after re-subscribing, since `set` and `update` + // are created anew each time. + const unsubscribe_2 = length.subscribe(noop); + if (typeof do_later === 'function') do_later(); + assert.equal(get(length_delayed, true), 0); + + unsubscribe_2(); + }); + it('derived dependency does not update and shared ancestor updates', () => { const root = writable({ a: 0, b: 0 }); const values: string[] = []; - // @ts-expect-error TODO feels like inference should work here const a = derived(root, ($root) => { return 'a' + $root.a; }); - // @ts-expect-error TODO feels like inference should work here const b = derived([a, root], ([$a, $root]) => { return 'b' + $root.b + $a; }); @@ -431,7 +652,6 @@ describe('derived', () => { const number = writable(1); - // @ts-expect-error TODO feels like inference should work here const numbers = derived(number, ($number) => { arr[0] = $number; return arr; @@ -451,17 +671,16 @@ describe('derived', () => { unsubscribe(); }); - it('calls a `cleanup` function', () => { + it('calls an `on_stop` function', () => { const num = writable(1); const values: number[] = []; const cleaned_up: number[] = []; - // @ts-expect-error TODO feels like inference should work here const d = derived(num, ($num, set) => { set($num * 2); - return function cleanup() { + return function on_stop() { cleaned_up.push($num); }; }); @@ -488,7 +707,10 @@ describe('derived', () => { const values: number[] = []; - // @ts-expect-error TODO feels like inference should work here + // @ts-expect-error Returning a non-function value from `derive_value` forces TypeScript to + // assume the function is a `SimpleDeriveValue` as opposed to a `ComplexDeriveValue`. Since the value will be discarded by the implementation, showing a type error is + // justified. const d = derived(num, ($num, set) => { set($num * 2); return {}; @@ -508,31 +730,10 @@ describe('derived', () => { unsubscribe(); }); - it('allows derived with different types', () => { - const a = writable('one'); - const b = writable(1); - - // @ts-expect-error TODO feels like inference should work here - const c = derived([a, b], ([a, b]) => `${a} ${b}`); - - assert.deepEqual(get(c), 'one 1'); - - a.set('two'); - b.set(2); - assert.deepEqual(get(c), 'two 2'); - }); - - it('works with RxJS-style observables', () => { - // @ts-expect-error TODO feels like inference should work here - const d = derived(fake_observable, (_) => _); - assert.equal(get(d), 42); - }); - it('does not restart when unsubscribed from another store with a shared ancestor', () => { const a = writable(true); let b_started = false; - // @ts-expect-error TODO feels like inference should work here const b = derived(a, (_, __) => { b_started = true; return () => { @@ -541,7 +742,6 @@ describe('derived', () => { }; }); - // @ts-expect-error TODO feels like inference should work here const c = derived(a, ($a, set) => { if ($a) return b.subscribe(set); }); @@ -550,23 +750,6 @@ describe('derived', () => { a.set(false); assert.equal(b_started, false); }); - - it('errors on undefined stores #1', () => { - assert.throws(() => { - // @ts-expect-error TODO feels like inference should work here - derived(null, (n) => n); - }); - }); - - it('errors on undefined stores #2', () => { - assert.throws(() => { - const a = writable(1); - // @ts-expect-error TODO feels like inference should work here - derived([a, null, undefined], ([n]) => { - return n * 2; - }); - }); - }); }); describe('get', () => { @@ -592,7 +775,8 @@ describe('readonly', () => { assert.equal(get(readable_store), 2); assert.equal(get(readable_store), get(writable_store)); - // @ts-ignore + // @ts-expect-error This should create a type errors, but this code is testing that the + // implementation also throws an error. assert.throws(() => readable_store.set(3)); }); }); From a06a4e609fda7edea2674c4638c878f1b904a4d7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 31 Jan 2024 17:18:54 -0500 Subject: [PATCH 4/5] revert some unnecessary changes, to reduce diff size --- .../svelte/src/internal/client/runtime.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/motion/spring.js | 5 +-- packages/svelte/src/motion/tweened.js | 7 ++--- packages/svelte/src/store/index.js | 24 -------------- packages/svelte/src/store/utils.js | 31 +++++++++++++++++++ 6 files changed, 37 insertions(+), 34 deletions(-) create mode 100644 packages/svelte/src/store/utils.js diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 820a5afe3135..dbd7484fa14e 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -1,5 +1,5 @@ -import { subscribe_to_store } from '../../store/index.js'; import { DEV } from 'esm-env'; +import { subscribe_to_store } from '../../store/utils.js'; import { noop, run_all } from '../common.js'; import { array_prototype, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 5d8e85ed56b2..12d9a3c130f4 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -1,7 +1,7 @@ import * as $ from '../client/runtime.js'; import { set_is_ssr } from '../client/runtime.js'; import { is_promise, noop } from '../common.js'; -import { subscribe_to_store } from '../../store/index.js'; +import { subscribe_to_store } from '../../store/utils.js'; import { DOMBooleanAttributes } from '../../constants.js'; export * from '../client/validate.js'; diff --git a/packages/svelte/src/motion/spring.js b/packages/svelte/src/motion/spring.js index e494650ee630..ec56d354109a 100644 --- a/packages/svelte/src/motion/spring.js +++ b/packages/svelte/src/motion/spring.js @@ -48,10 +48,7 @@ function tick_spring(ctx, last_value, current_value, target_value) { } /** - * The spring function in Svelte creates a store whose value is animated, with a motion that - * simulates the behavior of a spring. This means when the value changes, instead of transitioning - * at a steady rate, it "bounces" like a spring would, depending on the physics parameters provided. - * This adds a level of realism to the transitions and can enhance the user experience. + * The spring function in Svelte creates a store whose value is animated, with a motion that simulates the behavior of a spring. This means when the value changes, instead of transitioning at a steady rate, it "bounces" like a spring would, depending on the physics parameters provided. This adds a level of realism to the transitions and can enhance the user experience. * * https://svelte.dev/docs/svelte-motion#spring * @template [T=any] diff --git a/packages/svelte/src/motion/tweened.js b/packages/svelte/src/motion/tweened.js index 2b252f9951c3..d1e92b517867 100644 --- a/packages/svelte/src/motion/tweened.js +++ b/packages/svelte/src/motion/tweened.js @@ -71,8 +71,7 @@ function get_interpolator(a, b) { } /** - * A tweened store in Svelte is a special type of store that provides smooth transitions between - * state values over time. + * A tweened store in Svelte is a special type of store that provides smooth transitions between state values over time. * * https://svelte.dev/docs/svelte-motion#tweened * @template T @@ -125,9 +124,9 @@ export function tweened(value, defaults = {}) { if (now < start) return true; if (!started) { fn = interpolate(/** @type {any} */ (value), new_value); - if (typeof duration === 'function') { + if (typeof duration === 'function') duration = duration(/** @type {any} */ (value), new_value); - } + started = true; } if (previous_task) { diff --git a/packages/svelte/src/store/index.js b/packages/svelte/src/store/index.js index a51b5f13ed84..0c085a39b01c 100644 --- a/packages/svelte/src/store/index.js +++ b/packages/svelte/src/store/index.js @@ -501,27 +501,3 @@ export function get_store_value(store, allow_stale = false) { } export { get_store_value as get }; - -/** - * @template T - * @param {import('./public.js').Readable | import('./public.js').ExternalReadable | null | undefined} store - * @param {(value: T) => void} run - * @param {(value: T) => void} [invalidate] - * @returns {() => void} - */ -export function subscribe_to_store(store, run, invalidate) { - if (store == null) { - // @ts-expect-error - run(undefined); - - // @ts-expect-error - if (invalidate) invalidate(undefined); - - return noop; - } - - // @ts-expect-error `SubscriberInvalidator` is not public. - const unsubscribe = store.subscribe([run, invalidate]); - const is_rxjs = typeof unsubscribe !== 'function'; - return is_rxjs ? unsubscribe.unsubscribe : unsubscribe; -} diff --git a/packages/svelte/src/store/utils.js b/packages/svelte/src/store/utils.js new file mode 100644 index 000000000000..9bcc914fd138 --- /dev/null +++ b/packages/svelte/src/store/utils.js @@ -0,0 +1,31 @@ +import { noop } from '../internal/common.js'; + +/** + * @template T + * @param {import('./public').Readable | null | undefined} store + * @param {(value: T) => void} run + * @param {(value: T) => void} [invalidate] + * @returns {() => void} + */ +export function subscribe_to_store(store, run, invalidate) { + if (store == null) { + // @ts-expect-error + run(undefined); + + // @ts-expect-error + if (invalidate) invalidate(undefined); + + return noop; + } + + // Svelte store takes a private second argument + const unsub = store.subscribe( + run, + // @ts-expect-error + invalidate + ); + + // Also support RxJS + // @ts-expect-error TODO fix this in the types? + return unsub.unsubscribe ? () => unsub.unsubscribe() : unsub; +} From 2dec711f2f3e52c7060f353b9bba9510c601380b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 31 Jan 2024 17:19:38 -0500 Subject: [PATCH 5/5] missed a spot --- packages/svelte/src/motion/tweened.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/motion/tweened.js b/packages/svelte/src/motion/tweened.js index d1e92b517867..90b6aaa0073f 100644 --- a/packages/svelte/src/motion/tweened.js +++ b/packages/svelte/src/motion/tweened.js @@ -126,7 +126,6 @@ export function tweened(value, defaults = {}) { fn = interpolate(/** @type {any} */ (value), new_value); if (typeof duration === 'function') duration = duration(/** @type {any} */ (value), new_value); - started = true; } if (previous_task) {