Merge pull request #25 from udecode/bug

Add tests and bug fixes
udecode · Feb 6, 2025 · b5ff55d · b5ff55d
2 parents baa1ebc + 6643d02
commit b5ff55d
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 23 deletions.
diff --git a/.changeset/bright-coats-peel.md b/.changeset/bright-coats-peel.md
@@ -0,0 +1,5 @@
+---
+'jotai-x': patch
+---
+
+Add test cases and fix bugs
diff --git a/README.md b/README.md
@@ -153,8 +153,7 @@ const [stars, setStars] = useAppState('stars');
 // With selector and deps
 const upperName = useAppValue('name', {
   selector: (name) => name.toUpperCase(),
-  deps: []
-});
+}, []);
 ```
 
 #### 2. Store Instance Methods
@@ -204,14 +203,14 @@ const name = useAppValue('name');
 // With selector
 const upperName = useAppValue('name', {
   selector: (name) => name.toUpperCase(),
-  deps: [] // Optional deps array
-});
+}, [] // if selector is not memoized, provide deps array
+);
 
 // With equality function
 const name = useAppValue('name', {
   selector: (name) => name,
   equalityFn: (prev, next) => prev.length === next.length
-});
+}, []);
 ```
 
 #### `use<Name>Set(key)`
@@ -404,7 +403,7 @@ const selector = useCallback((name) => name.toUpperCase(), []);
 useUserValue('name', { selector });
 
 // ✅ Correct - provide deps array
-useUserValue('name', { selector: (name) => name.toUpperCase(), deps: [] });
+useUserValue('name', { selector: (name) => name.toUpperCase() }, []);
 
 // ✅ Correct - no selector
 useUserValue('name');

diff --git a/packages/jotai-x/README.md b/packages/jotai-x/README.md
@@ -153,8 +153,7 @@ const [stars, setStars] = useAppState('stars');
 // With selector and deps
 const upperName = useAppValue('name', {
   selector: (name) => name.toUpperCase(),
-  deps: []
-});
+}, []);
 ```
 
 #### 2. Store Instance Methods
@@ -204,14 +203,14 @@ const name = useAppValue('name');
 // With selector
 const upperName = useAppValue('name', {
   selector: (name) => name.toUpperCase(),
-  deps: [] // Optional deps array
-});
+}, [] // if selector is not memoized, provide deps array
+);
 
 // With equality function
 const name = useAppValue('name', {
   selector: (name) => name,
   equalityFn: (prev, next) => prev.length === next.length
-});
+}, []);
 ```
 
 #### `use<Name>Set(key)`
@@ -404,7 +403,7 @@ const selector = useCallback((name) => name.toUpperCase(), []);
 useUserValue('name', { selector });
 
 // ✅ Correct - provide deps array
-useUserValue('name', { selector: (name) => name.toUpperCase(), deps: [] });
+useUserValue('name', { selector: (name) => name.toUpperCase() }, []);
 
 // ✅ Correct - no selector
 useUserValue('name');

diff --git a/packages/jotai-x/src/createAtomStore.spec.tsx b/packages/jotai-x/src/createAtomStore.spec.tsx
@@ -29,10 +29,14 @@ describe('createAtomStore', () => {
       arr: INITIAL_ARR,
     };
 
-    const { useMyTestStoreStore, MyTestStoreProvider } = createAtomStore(
-      initialTestStoreValue,
-      { name: 'myTestStore' as const }
-    );
+    const {
+      myTestStoreStore,
+      useMyTestStoreStore,
+      MyTestStoreProvider,
+      useMyTestStoreValue,
+    } = createAtomStore(initialTestStoreValue, {
+      name: 'myTestStore' as const,
+    });
 
     let numRenderCount = 0;
     const NumRenderer = () => {
@@ -92,6 +96,24 @@ describe('createAtomStore', () => {
       );
     };
 
+    let arrNumRenderCountWithOneHook = 0;
+    const ArrNumRendererWithOneHook = () => {
+      arrNumRenderCountWithOneHook += 1;
+      const num = useMyTestStoreValue('num');
+      const arrNum = useMyTestStoreValue(
+        'arr',
+        {
+          selector: (v) => v[num],
+        },
+        [num]
+      );
+      return (
+        <div>
+          <div>arrNumWithOneHook: {arrNum}</div>
+        </div>
+      );
+    };
+
     let arrNumRenderWithDepsCount = 0;
     const ArrNumRendererWithDeps = () => {
       arrNumRenderWithDepsCount += 1;
@@ -105,11 +127,31 @@ describe('createAtomStore', () => {
       );
     };
 
+    let arrNumRenderWithDepsAndAtomCount = 0;
+    const ArrNumRendererWithDepsAndAtom = () => {
+      arrNumRenderWithDepsAndAtomCount += 1;
+      const store = useMyTestStoreStore();
+      const numAtom = myTestStoreStore.atom.num;
+      const num = store.useAtomValue(numAtom);
+      const arrAtom = myTestStoreStore.atom.arr;
+      const arrNum = store.useAtomValue(arrAtom, (v) => v[num], [num]);
+      return (
+        <div>
+          <div>arrNumWithDepsAndAtom: {arrNum}</div>
+        </div>
+      );
+    };
+
     const BadSelectorRenderer = () => {
       const arr0 = useMyTestStoreStore().useArrValue((v) => v[0]);
       return <div>{arr0}</div>;
     };
 
+    const BadSelectorRenderer2 = () => {
+      const arr0 = useMyTestStoreValue('arr', { selector: (v) => v[0] });
+      return <div>{arr0}</div>;
+    };
+
     const Buttons = () => {
       const store = useMyTestStoreStore();
       return (
@@ -154,7 +196,9 @@ describe('createAtomStore', () => {
           <Arr0Renderer />
           <Arr1Renderer />
           <ArrNumRenderer />
+          <ArrNumRendererWithOneHook />
           <ArrNumRendererWithDeps />
+          <ArrNumRendererWithDepsAndAtom />
           <Buttons />
         </MyTestStoreProvider>
       );
@@ -166,7 +210,9 @@ describe('createAtomStore', () => {
       expect(arr0RenderCount).toBe(2);
       expect(arr1RenderCount).toBe(2);
       expect(arrNumRenderCount).toBe(2);
+      expect(arrNumRenderCountWithOneHook).toBe(2);
       expect(arrNumRenderWithDepsCount).toBe(2);
+      expect(arrNumRenderWithDepsAndAtomCount).toBe(2);
       expect(getByText('arrNum: alice')).toBeInTheDocument();
       expect(getByText('arrNumWithDeps: alice')).toBeInTheDocument();
 
@@ -177,7 +223,9 @@ describe('createAtomStore', () => {
       expect(arr0RenderCount).toBe(2);
       expect(arr1RenderCount).toBe(2);
       expect(arrNumRenderCount).toBe(5);
+      expect(arrNumRenderCountWithOneHook).toBe(5);
       expect(arrNumRenderWithDepsCount).toBe(5);
+      expect(arrNumRenderWithDepsAndAtomCount).toBe(5);
       expect(getByText('arrNum: bob')).toBeInTheDocument();
       expect(getByText('arrNumWithDeps: bob')).toBeInTheDocument();
 
@@ -188,7 +236,9 @@ describe('createAtomStore', () => {
       expect(arr0RenderCount).toBe(2);
       expect(arr1RenderCount).toBe(2);
       expect(arrNumRenderCount).toBe(5);
+      expect(arrNumRenderCountWithOneHook).toBe(5);
       expect(arrNumRenderWithDepsCount).toBe(5);
+      expect(arrNumRenderWithDepsAndAtomCount).toBe(5);
       expect(getByText('arrNum: bob')).toBeInTheDocument();
       expect(getByText('arrNumWithDeps: bob')).toBeInTheDocument();
 
@@ -199,7 +249,9 @@ describe('createAtomStore', () => {
       expect(arr0RenderCount).toBe(2);
       expect(arr1RenderCount).toBe(2);
       expect(arrNumRenderCount).toBe(5);
+      expect(arrNumRenderCountWithOneHook).toBe(5);
       expect(arrNumRenderWithDepsCount).toBe(5);
+      expect(arrNumRenderWithDepsAndAtomCount).toBe(5);
       expect(getByText('arrNum: bob')).toBeInTheDocument();
       expect(getByText('arrNumWithDeps: bob')).toBeInTheDocument();
 
@@ -210,7 +262,9 @@ describe('createAtomStore', () => {
       expect(arr0RenderCount).toBe(3);
       expect(arr1RenderCount).toBe(2);
       expect(arrNumRenderCount).toBe(8);
+      expect(arrNumRenderCountWithOneHook).toBe(8);
       expect(arrNumRenderWithDepsCount).toBe(8);
+      expect(arrNumRenderWithDepsAndAtomCount).toBe(8);
       expect(getByText('arrNum: ava')).toBeInTheDocument();
       expect(getByText('arrNumWithDeps: ava')).toBeInTheDocument();
     });
@@ -224,6 +278,16 @@ describe('createAtomStore', () => {
         )
       ).toThrow();
     });
+
+    it('Throw error is user does memoize selector 2', () => {
+      expect(() =>
+        render(
+          <MyTestStoreProvider>
+            <BadSelectorRenderer2 />
+          </MyTestStoreProvider>
+        )
+      ).toThrow();
+    });
   });
 
   describe('single provider', () => {

diff --git a/packages/jotai-x/src/createAtomStore.ts b/packages/jotai-x/src/createAtomStore.ts
@@ -493,10 +493,10 @@ export interface CreateAtomStoreOptions<
  * Each property will have a getter and setter.
  *
  * @example
- * const { exampleStore, useExampleStore } = createAtomStore({ count: 1, say: 'hello' }, { name: 'example' as const })
- * const [count, setCount] = useExampleStore().useCountState()
- * const say = useExampleStore().useSayValue()
- * const setSay = useExampleStore().useSetSay()
+ * const { exampleStore, useExampleStore, useExampleValue, useExampleState, useExampleSet } = createAtomStore({ count: 1, say: 'hello' }, { name: 'example' as const })
+ * const [count, setCount] = useExampleState()
+ * const say = useExampleValue('say')
+ * const setSay = useExampleSet('say')
  * setSay('world')
  * const countAtom = exampleStore.atom.count
  */
@@ -591,7 +591,7 @@ export const createAtomStore = <
       if (renderCount > infiniteRenderDetectionLimit) {
         throw new Error(
           `
-use<Key>Value/useValue has rendered ${infiniteRenderDetectionLimit} times in the same render.
+use<Key>Value/useValue/use<StoreName>Value has rendered ${infiniteRenderDetectionLimit} times in the same render.
 It is very likely to have fallen into an infinite loop.
 That is because you do not memoize the selector/equalityFn function param.
 Please wrap them with useCallback or configure the deps array correctly.`
@@ -898,8 +898,8 @@ Please wrap them with useCallback or configure the deps array correctly.`
       store,
       options,
       selector as any,
-      equalityFn as any,
-      deps
+      equalityFn ?? deps,
+      equalityFn && deps
     );
   };