Skip to content

Commit

Permalink
fix: do not call equal with an uninitialized value
Browse files Browse the repository at this point in the history
  • Loading branch information
divdavem committed Oct 4, 2023
1 parent 4b8f678 commit 9ac0756
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 2 deletions.
39 changes: 39 additions & 0 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,27 @@ describe('stores', () => {
expect(get(multiplyStore)).toBe(14);
});

it('should not call equal with undefined during the first computation', () => {
const a = writable(1);
const equal = vi.fn(Object.is);
const b = derived([a], {
derive([a]) {
return a + 1;
},
equal,
});
const values: number[] = [];
const unsubscribe = b.subscribe((value) => {
values.push(value);
});
expect(equal).not.toHaveBeenCalled();
a.set(2);
expect(equal).toHaveBeenCalledOnce();
expect(equal).toHaveBeenCalledWith(2, 3);
equal.mockClear();
unsubscribe();
});

it('should allow calling a derived store as a function', () => {
const numbersStore = writable(0);
const multiplyStore = derived([numbersStore], (values) => values[0] * 2);
Expand Down Expand Up @@ -2579,6 +2600,24 @@ describe('stores', () => {
});

describe('computed', () => {
it('should not call equal with undefined during the first computation', () => {
const a = writable(1);
const equal = vi.fn(Object.is);
const b = computed(() => a() + 1, {
equal,
});
const values: number[] = [];
const unsubscribe = b.subscribe((value) => {
values.push(value);
});
expect(equal).not.toHaveBeenCalled();
a.set(2);
expect(equal).toHaveBeenCalledOnce();
expect(equal).toHaveBeenCalledWith(2, 3);
equal.mockClear();
unsubscribe();
});

it('should properly subscribe to used stores and unsubscribe from unused ones', () => {
const a = writable(true);
let bHasListeners = false;
Expand Down
19 changes: 17 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ const createEqualCache = (valueIndex: number): Record<number, boolean> => ({
0: false, // the subscriber never received any value
});

const skipEqualInSet = Symbol();

/**
* Base class that can be extended to easily create a custom {@link Readable} store.
*
Expand Down Expand Up @@ -408,6 +410,8 @@ export abstract class Store<T> implements Readable<T> {
#equalCache = createEqualCache(1);
#oldSubscriptions = new WeakMap<Unsubscriber, PrivateSubscriberObject<T>>();

private [skipEqualInSet] = false;

/**
*
* @param value - Initial value of the store
Expand Down Expand Up @@ -455,7 +459,7 @@ export abstract class Store<T> implements Readable<T> {
const value = this.#value;
let equal = equalCache[subscriber._valueIndex];
if (equal == null) {
equal = this.equal(subscriber._value, value);
equal = !!this.equal(subscriber._value, value);
equalCache[subscriber._valueIndex] = equal;
}
subscriber._valueIndex = valueIndex;
Expand Down Expand Up @@ -567,11 +571,15 @@ export abstract class Store<T> implements Readable<T> {
* @param value - value to be used as the new state of a store.
*/
protected set(value: T): void {
if (!this.equal(this.#value, value)) {
const skipEqual = this[skipEqualInSet];
if (skipEqual || !this.equal(this.#value, value)) {
const valueIndex = this.#valueIndex + 1;
this.#valueIndex = valueIndex;
this.#value = value;
this.#equalCache = createEqualCache(valueIndex);
if (skipEqual) {
delete this.#equalCache[valueIndex - 1];
}
this.pauseSubscribers();
}
this.resumeSubscribers();
Expand Down Expand Up @@ -1030,8 +1038,13 @@ export function derived<T, S extends StoresInput>(
const { derive, ...opts } = options;
const Derived = isSyncDeriveFn(derive)
? class extends DerivedStore<T, S> {
constructor(stores: S, initialValue: T) {
super(stores, initialValue);
this[skipEqualInSet] = true; // skip call to equal in set until the first value is set
}
protected override derive(values: StoresInputValues<S>) {
this.set(derive(values));
this[skipEqualInSet] = false;
}
}
: class extends DerivedStore<T, S> {
Expand Down Expand Up @@ -1091,6 +1104,7 @@ abstract class ComputedStore<T> extends Store<T> {

constructor() {
super(undefined as T);
this[skipEqualInSet] = true; // skip call to equal in set until the first value is set
}

#createSubscription<T>(subscribe: Readable<T>['subscribe']) {
Expand Down Expand Up @@ -1167,6 +1181,7 @@ abstract class ComputedStore<T> extends Store<T> {
reactiveContext = previousReactiveContext;
}
this.set(value);
this[skipEqualInSet] = false;
for (const [store, info] of this.#subscriptions) {
if (info.versionIndex !== versionIndex) {
this.#subscriptions.delete(store);
Expand Down

0 comments on commit 9ac0756

Please sign in to comment.