From 43c75e257409fd25a9c17fb37a4e880238a5276c Mon Sep 17 00:00:00 2001
From: Ben Durrant <ben.j.durrant@gmail.com>
Date: Sat, 3 Jun 2023 17:27:38 +0100
Subject: [PATCH] add test

---
 src/hooks/useSelector.ts        |   2 -
 test/hooks/useSelector.spec.tsx | 105 +++++++++++++++++++++-----------
 2 files changed, 68 insertions(+), 39 deletions(-)

diff --git a/src/hooks/useSelector.ts b/src/hooks/useSelector.ts
index 0415e908b..e4c232e7d 100644
--- a/src/hooks/useSelector.ts
+++ b/src/hooks/useSelector.ts
@@ -90,7 +90,6 @@ export function createSelectorHook(context = ReactReduxContext): UseSelector {
           const selected = selector(state)
           if (process.env.NODE_ENV !== 'production') {
             const finalStabilityCheck =
-              // are we safe to use ?? here?
               typeof stabilityCheck === 'undefined'
                 ? globalStabilityCheck
                 : stabilityCheck
@@ -114,7 +113,6 @@ export function createSelectorHook(context = ReactReduxContext): UseSelector {
               }
             }
             const finalNoopCheck =
-              // are we safe to use ?? here?
               typeof noopCheck === 'undefined' ? globalNoopCheck : noopCheck
             if (
               finalNoopCheck === 'always' ||
diff --git a/test/hooks/useSelector.spec.tsx b/test/hooks/useSelector.spec.tsx
index 20d1e5464..500aded64 100644
--- a/test/hooks/useSelector.spec.tsx
+++ b/test/hooks/useSelector.spec.tsx
@@ -28,13 +28,19 @@ import type { FunctionComponent, DispatchWithoutAction, ReactNode } from 'react'
 import type { Store, AnyAction } from 'redux'
 import { UseSelectorOptions } from '../../src/hooks/useSelector'
 
-// most of these tests depend on selectors being run once, which stabilityCheck doesn't do
-// rather than specify it every time, let's make a new "default" here
+// disable checks by default
 function ProviderMock<A extends Action<any> = AnyAction, S = unknown>({
   stabilityCheck = 'never',
+  noopCheck = 'never',
   ...props
 }: ProviderProps<A, S>) {
-  return <Provider {...props} stabilityCheck={stabilityCheck} />
+  return (
+    <Provider
+      {...props}
+      stabilityCheck={stabilityCheck}
+      noopCheck={noopCheck}
+    />
+  )
 }
 
 const IS_REACT_18 = React.version.startsWith('18')
@@ -739,36 +745,38 @@ describe('React', () => {
       })
 
       describe('Development mode checks', () => {
+        const consoleSpy = jest
+          .spyOn(console, 'warn')
+          .mockImplementation(() => {})
+        afterEach(() => {
+          consoleSpy.mockClear()
+        })
+        afterAll(() => {
+          consoleSpy.mockRestore()
+        })
+
+        const RenderSelector = ({
+          selector,
+          options,
+        }: {
+          selector: (state: NormalStateType) => unknown
+          options?: UseSelectorOptions<unknown>
+        }) => {
+          useSelector(selector, options)
+          return null
+        }
         describe('selector result stability check', () => {
           const selector = jest.fn((state: NormalStateType) => state.count)
 
-          const consoleSpy = jest
-            .spyOn(console, 'warn')
-            .mockImplementation(() => {})
           afterEach(() => {
-            consoleSpy.mockClear()
             selector.mockClear()
           })
-          afterAll(() => {
-            consoleSpy.mockRestore()
-          })
-
-          const RenderSelector = ({
-            selector,
-            options,
-          }: {
-            selector: (state: NormalStateType) => unknown
-            options?: UseSelectorOptions<unknown>
-          }) => {
-            useSelector(selector, options)
-            return null
-          }
 
           it('calls a selector twice, and warns in console if it returns a different result', () => {
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector selector={selector} />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(2)
@@ -780,9 +788,9 @@ describe('React', () => {
             const unstableSelector = jest.fn(() => Math.random())
 
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector selector={unstableSelector} />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(2)
@@ -806,12 +814,12 @@ describe('React', () => {
             }))
 
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector
                   selector={unstableSelector}
                   options={{ equalityFn: shallowEqual }}
                 />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(unstableSelector).toHaveBeenCalledTimes(2)
@@ -819,9 +827,9 @@ describe('React', () => {
           })
           it('by default will only check on first selector call', () => {
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector selector={selector} />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(2)
@@ -834,9 +842,9 @@ describe('React', () => {
           })
           it('disables check if context or hook specifies', () => {
             rtl.render(
-              <Provider store={normalStore} stabilityCheck="never">
+              <ProviderMock store={normalStore} stabilityCheck="never">
                 <RenderSelector selector={selector} />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(1)
@@ -846,21 +854,21 @@ describe('React', () => {
             selector.mockClear()
 
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector
                   selector={selector}
                   options={{ stabilityCheck: 'never' }}
                 />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(1)
           })
           it('always runs check if context or hook specifies', () => {
             rtl.render(
-              <Provider store={normalStore} stabilityCheck="always">
+              <ProviderMock store={normalStore} stabilityCheck="always">
                 <RenderSelector selector={selector} />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(2)
@@ -876,12 +884,12 @@ describe('React', () => {
             selector.mockClear()
 
             rtl.render(
-              <Provider store={normalStore}>
+              <ProviderMock stabilityCheck="once" store={normalStore}>
                 <RenderSelector
                   selector={selector}
                   options={{ stabilityCheck: 'always' }}
                 />
-              </Provider>
+              </ProviderMock>
             )
 
             expect(selector).toHaveBeenCalledTimes(2)
@@ -893,6 +901,29 @@ describe('React', () => {
             expect(selector).toHaveBeenCalledTimes(4)
           })
         })
+        describe('no-op selector check', () => {
+          it('warns for selectors that return the entire root state', () => {
+            rtl.render(
+              <ProviderMock noopCheck="once" store={normalStore}>
+                <RenderSelector selector={(state) => state.count} />
+              </ProviderMock>
+            )
+
+            expect(consoleSpy).not.toHaveBeenCalled()
+
+            rtl.cleanup()
+
+            rtl.render(
+              <ProviderMock noopCheck="once" store={normalStore}>
+                <RenderSelector selector={(state) => state} />
+              </ProviderMock>
+            )
+
+            expect(consoleSpy).toHaveBeenCalledWith(
+              expect.stringContaining('returned the root state when called.')
+            )
+          })
+        })
       })
     })