Skip to content

feat: pass update function to store setup callbacks #6750

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

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
31 changes: 24 additions & 7 deletions site/content/docs/03-run-time.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ This makes it possible to wrap almost any other reactive state handling library
store = writable(value?: any)
```
```js
store = writable(value?: any, start?: (set: (value: any) => void) => () => void)
store = writable(value?: any, start?: (set: (value: any) => void, update?: (fn: any => any) => void) => () => void)
```

---
Expand All @@ -295,7 +295,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. It must return a `stop` function that is called when the subscriber count goes from one to zero.
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 optionally 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
import { writable } from 'svelte/store';
Expand Down Expand Up @@ -338,6 +338,16 @@ const time = readable(null, set => {

return () => clearInterval(interval);
});

const ticktock = readable(null, (set, update) => {
set('tick');

const interval = setInterval(() => {
update(sound => sound === 'tick' ? 'tock' : 'tick');
}, 1000);

return () => clearInterval(interval);
});
```

#### `derived`
Expand All @@ -346,13 +356,13 @@ const time = readable(null, set => {
store = derived(a, callback: (a: any) => any)
```
```js
store = derived(a, callback: (a: any, set: (value: any) => void) => void | () => void, initial_value: any)
store = derived(a, callback: (a: any, set: (value: any) => void, update?: (fn: any => any) => void) => void | () => void, initial_value: any)
```
```js
store = derived([a, ...b], callback: ([a: any, ...b: any[]]) => any)
```
```js
store = derived([a, ...b], callback: ([a: any, ...b: any[]], set: (value: any) => void) => void | () => void, initial_value: any)
store = derived([a, ...b], callback: ([a: any, ...b: any[]], set: (value: any) => void, update?: (fn: any => any) => void) => void | () => void, initial_value: any)
```

---
Expand All @@ -369,16 +379,23 @@ const doubled = derived(a, $a => $a * 2);

---

The callback can set a value asynchronously by accepting a second argument, `set`, and calling it when appropriate.
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.

In this case, you can also pass a third argument to `derived` — the initial value of the derived store before `set` is first called.
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`.

```js
import { derived } from 'svelte/store';

const delayed = derived(a, ($a, set) => {
setTimeout(() => set($a), 1000);
}, 'one moment...');

const delayedIncrement = derived(a, ($a, set, update) => {
set($a);
setTimeout(() => update(x => x + 1), 1000);
// every time $a produces a value, this produces two
// values, $a immediately and then $a + 1 a second later
});
```

---
Expand Down Expand Up @@ -808,7 +825,7 @@ The `crossfade` function creates a pair of [transitions](docs#transition_fn) cal
* `delay` (`number`, default 0) — milliseconds before starting
* `duration` (`number` | `function`, default 800) — milliseconds the transition lasts
* `easing` (`function`, default `cubicOut`) — an [easing function](docs#svelte_easing)
* `fallback` (`function`) — A fallback [transition](docs#transition_fn) to use for send when there is no matching element being received, and for receive when there is no element being sent.
* `fallback` (`function`) — A fallback [transition](docs#transition_fn) to use for send when there is no matching element being received, and for receive when there is no element being sent.

```sv
<script>
Expand Down
10 changes: 5 additions & 5 deletions src/runtime/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type Updater<T> = (value: T) => T;
type Invalidator<T> = (value?: T) => void;

/** Start and stop notification callbacks. */
export type StartStopNotifier<T> = (set: Subscriber<T>) => Unsubscriber | void;
export type StartStopNotifier<T> = (set: Subscriber<T>, update?: (fn: Updater<T>) => void) => Unsubscriber | void;
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be a non-optional parameter, else TS will error here if you try to use update without checking for its existance first.
Which makes me think: Is this a breaking change for people implementing their own store contract? If so, yeah it needs to be an optional update parameter. @Conduitry what's your stance on this?

If we leave it like this, this means we need to adjust the other signatures a bit to type them so that update is not optional as it is in fact provided for readable/writable/derived

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange, the two unit tests I added ran just fine, and I'm not seeing any TS errors in VS code on my use of update in the tests. I thought it would be a breaking change (at least from Typescript's perspective if not from Javascript's perspective) to make this non-optional as then everyone who has written readable(0, (set) => { }); would get Typescript complaining that they didn't take the second parameter. But then, I don't know all the subtleties of Typescript; maybe it's fine with omitting parameters to functions because Javascript allows that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, at no point in the Svelte code does the update function from StartStopNotifier get called; it's only passed to callbacks. And the callbacks can either take the update parameter and use it, or not take it in which case it would obviously be an error for them to have a call to update(x => x+1). But I don't think making this optional will actually produce any Typescript errors, and I do think that making it non-optional will be a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

For function parameters, the inverse is true: If the function expects a function-parameter with x arguments, you can pass in a function with zero to x parameters.
You don't see errors because the repository runs TS in non-strict mode (no strict null checks).
See this playground for more: https://www.typescriptlang.org/play?#code/GYVwdgxgLglg9mABACwKYBt1wBTDALkWwCdUBHQgZymJjAHMAaROABygH4qa76BKRAF4AfIgBucGABMBAbwC+AKEVpMObAJGIFfANyIA9AcRg4iVsTgAjdKgC2KjFmwlyzNlE2jZixH5bsAHRQcAAycADuqMQAwgCGlKga+kaI0ZbEiFaoEHEgiQFQiBBwIOhSWaiI4FKowHSoUoryfEA

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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


/** Readable interface for subscribing. */
export interface Readable<T> {
Expand Down Expand Up @@ -92,7 +92,7 @@ export function writable<T>(value?: T, start: StartStopNotifier<T> = noop): Writ
const subscriber: SubscribeInvalidateTuple<T> = [run, invalidate];
subscribers.add(subscriber);
if (subscribers.size === 1) {
stop = start(set) || noop;
stop = start(set, update) || noop;
}
run(value);

Expand Down Expand Up @@ -125,7 +125,7 @@ type StoresValues<T> = T extends Readable<infer U> ? U :
*/
export function derived<S extends Stores, T>(
stores: S,
fn: (values: StoresValues<S>, set: (value: T) => void) => Unsubscriber | void,
fn: (values: StoresValues<S>, set: Subscriber<T>, update?: (fn: Updater<T>) => void) => Unsubscriber | void,
initial_value?: T
): Readable<T>;

Expand Down Expand Up @@ -163,7 +163,7 @@ export function derived<T>(stores: Stores, fn: Function, initial_value?: T): Rea

const auto = fn.length < 2;

return readable(initial_value, (set) => {
return readable(initial_value, (set, update) => {
let inited = false;
const values = [];

Expand All @@ -175,7 +175,7 @@ export function derived<T>(stores: Stores, fn: Function, initial_value?: T): Rea
return;
}
cleanup();
const result = fn(single ? values[0] : values, set);
const result = fn(single ? values[0] : values, set, update);
if (auto) {
set(result as T);
} else {
Expand Down
77 changes: 77 additions & 0 deletions test/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,50 @@ describe('store', () => {
assert.deepEqual(values, [0, 1, 2]);
});

it('passes an optional update function', () => {
let running;
let tick;
let add;

const store = readable(undefined, (set, update) => {
tick = set;
running = true;
add = n => update(value => value + n);

set(0);

return () => {
tick = () => { };
add = _ => { };
running = false;
};
});

assert.ok(!running);

const values = [];

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

assert.ok(running);
tick(1);
tick(2);
add(3);
add(4);
tick(5);
add(6);

unsubscribe();

assert.ok(!running);
tick(7);
add(8);

assert.deepEqual(values, [0, 1, 2, 5, 9, 5, 11]);
});

it('creates an undefined readable store', () => {
const store = readable();
const values = [];
Expand Down Expand Up @@ -231,6 +275,39 @@ describe('store', () => {
assert.deepEqual(values, [0, 2, 4]);
});

it('passes optional set and update functions', () => {
const number = writable(1);
const evensAndSquaresOf4 = derived(number, (n, set, update) => {
if (n % 2 === 0) set(n);
if (n % 4 === 0) update(n => n * n);
}, 0);

const values = [];

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

number.set(2);
number.set(3);
number.set(4);
number.set(5);
number.set(6);
assert.deepEqual(values, [0, 2, 4, 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]);

unsubscribe();

number.set(11);
number.set(12);
assert.deepEqual(values, [0, 2, 4, 16, 6, 8, 64, 10]);
});

it('prevents glitches', () => {
const lastname = writable('Jekyll');
const firstname = derived(lastname, n => n === 'Jekyll' ? 'Henry' : 'Edward');
Expand Down