From f6115d91d0cad4aeefe9445545ac33bc8a1a6015 Mon Sep 17 00:00:00 2001
From: David-Emmanuel DIVERNOIS <david-emmanuel.divernois@amadeus.com>
Date: Tue, 3 Oct 2023 11:38:28 +0200
Subject: [PATCH] fix: do not call equal with an uninitialized value

---
 src/index.spec.ts | 37 +++++++++++++++++++++++++++++++++++++
 src/index.ts      | 21 ++++++++++++++++++---
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/index.spec.ts b/src/index.spec.ts
index 60a1217..8c0a8f6 100644
--- a/src/index.spec.ts
+++ b/src/index.spec.ts
@@ -1086,6 +1086,26 @@ 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);
+      unsubscribe();
+    });
+
     it('should allow calling a derived store as a function', () => {
       const numbersStore = writable(0);
       const multiplyStore = derived([numbersStore], (values) => values[0] * 2);
@@ -2579,6 +2599,23 @@ 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);
+      unsubscribe();
+    });
+
     it('should properly subscribe to used stores and unsubscribe from unused ones', () => {
       const a = writable(true);
       let bHasListeners = false;
diff --git a/src/index.ts b/src/index.ts
index 54a22c8..694d9f8 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -364,10 +364,12 @@ export const get = <T>(store: StoreInput<T>): T => reactiveContext(store);
 const createEqualCache = (valueIndex: number): Record<number, boolean> => ({
   [valueIndex]: true, // the subscriber already has the last value
   [valueIndex - 1]: false, // the subscriber had the previous value,
-  // which is known to be different because notEqual is called in the set method
+  // which is known to be different because equal is called in the set method
   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.
  *
@@ -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
@@ -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;
@@ -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();
@@ -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> {
@@ -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']) {
@@ -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);