Add initial implementation for noopCheck

src/createSelectorCreator.ts

@@ -22,6 +22,7 @@ import {
   collectInputSelectorResults,
   ensureIsArray,
   getDependencies,
+  runNoopCheck,
   runStabilityCheck,
   shouldRunInputStabilityCheck
 } from './utils'
@@ -187,6 +188,14 @@ export function setInputStabilityCheckEnabled(
   globalStabilityCheck = inputStabilityCheckFrequency
 }
 
+let globalNoopCheck: StabilityCheckFrequency = 'once'
+
+export const setGlobalNoopCheck = (
+  noopCheckFrequency: StabilityCheckFrequency
+) => {
+  globalNoopCheck = noopCheckFrequency
+}
+
 /**
  * Creates a selector creator function with the specified memoization function and options for customizing memoization behavior.
  *
@@ -374,7 +383,8 @@ export function createSelectorCreator<
       memoizeOptions = [],
       argsMemoize = defaultMemoize,
       argsMemoizeOptions = [],
-      inputStabilityCheck = globalStabilityCheck
+      inputStabilityCheck = globalStabilityCheck,
+      noopCheck = globalNoopCheck
     } = combinedOptions
 
     // Simplifying assumption: it's unlikely that the first options arg of the provided memoizer
@@ -408,6 +418,14 @@ export function createSelectorCreator<
         arguments
       )
 
+      if (
+        process.env.NODE_ENV !== 'production' &&
+        inputSelectorResults.length &&
+        shouldRunInputStabilityCheck(noopCheck, firstRun)
+      ) {
+        runNoopCheck(resultFunc as Combiner<InputSelectors, Result>)
+      }
+
       if (
         process.env.NODE_ENV !== 'production' &&
         shouldRunInputStabilityCheck(inputStabilityCheck, firstRun)

src/index.ts

@@ -2,6 +2,7 @@ export { autotrackMemoize as unstable_autotrackMemoize } from './autotrackMemoiz
 export {
   createSelector,
   createSelectorCreator,
+  setGlobalNoopCheck,
   setInputStabilityCheckEnabled
 } from './createSelectorCreator'
 export type { CreateSelectorFunction } from './createSelectorCreator'

src/types.ts

@@ -83,6 +83,8 @@ export interface CreateSelectorOptions<
    */
   inputStabilityCheck?: StabilityCheckFrequency
 
+  noopCheck?: StabilityCheckFrequency
+
   /**
    * The memoize function that is used to memoize the {@linkcode OutputSelectorFields.resultFunc resultFunc}
    * inside `createSelector` (e.g., `defaultMemoize` or `weakMapMemoize`).

src/utils.ts

@@ -1,4 +1,5 @@
 import type {
+  AnyFunction,
   CreateSelectorOptions,
   Selector,
   SelectorArray,
@@ -164,6 +165,24 @@ export function runStabilityCheck(
   }
 }
 
+export const runNoopCheck = <Func extends AnyFunction>(resultFunc: Func) => {
+  let isInputSameAsOutput = false
+  try {
+    const emptyObject = {}
+    if (resultFunc(emptyObject) === emptyObject) isInputSameAsOutput = true
+  } catch {
+    // Do nothing
+  }
+  if (isInputSameAsOutput) {
+    console.warn(
+      'The result function returned its own inputs without modification. e.g' +
+        '\n`createSelector([state => state.todos], todos => todos)`' +
+        '\nThis could lead to inefficient memoization and unnecessary re-renders.' +
+        '\nEnsure transformation logic is in the result function, and extraction logic is in the input selectors.'
+    )
+  }
+}
+
 /**
  * Determines if the input stability check should run.
  *

test/defaultMemoize.spec.ts

@@ -269,12 +269,12 @@ describe('defaultMemoize', () => {
     )
 
     fooChangeHandler(state)
-    expect(fooChangeSpy.mock.calls.length).toEqual(1)
+    expect(fooChangeSpy.mock.calls.length).toEqual(2)
 
     // no change
     fooChangeHandler(state)
     // this would fail
-    expect(fooChangeSpy.mock.calls.length).toEqual(1)
+    expect(fooChangeSpy.mock.calls.length).toEqual(2)
 
     const state2 = { a: 1 }
     let count = 0
@@ -285,9 +285,9 @@ describe('defaultMemoize', () => {
     })
 
     selector(state)
-    expect(count).toBe(1)
+    expect(count).toBe(2)
     selector(state)
-    expect(count).toBe(1)
+    expect(count).toBe(2)
   })
 
   test('Accepts an options object as an arg', () => {
@@ -369,33 +369,33 @@ describe('defaultMemoize', () => {
 
     // Initial call
     selector('a') // ['a']
-    expect(funcCalls).toBe(1)
+    expect(funcCalls).toBe(2)
 
     // In cache - memoized
     selector('a') // ['a']
-    expect(funcCalls).toBe(1)
+    expect(funcCalls).toBe(2)
 
     // Added
     selector('b') // ['b', 'a']
-    expect(funcCalls).toBe(2)
+    expect(funcCalls).toBe(3)
 
     // Added
     selector('c') // ['c', 'b', 'a']
-    expect(funcCalls).toBe(3)
+    expect(funcCalls).toBe(4)
 
     // Already in cache
     selector('c') // ['c', 'b', 'a']
-    expect(funcCalls).toBe(3)
+    expect(funcCalls).toBe(4)
 
     selector.memoizedResultFunc.clearCache()
 
     // Added
     selector('a') // ['a']
-    expect(funcCalls).toBe(4)
+    expect(funcCalls).toBe(5)
 
     // Already in cache
     selector('a') // ['a']
-    expect(funcCalls).toBe(4)
+    expect(funcCalls).toBe(5)
 
     // make sure clearCache is passed to the selector correctly
     selector.clearCache()
@@ -404,7 +404,7 @@ describe('defaultMemoize', () => {
     // Note: the outer arguments wrapper function still has 'a' in its own size-1 cache, so passing
     // 'a' here would _not_ recalculate
     selector('b') // ['b']
-    expect(funcCalls).toBe(5)
+    expect(funcCalls).toBe(6)
 
     try {
       //@ts-expect-error issue 591

test/noopCheck.test.ts

@@ -0,0 +1,127 @@
+import { createSelector, setGlobalNoopCheck } from 'reselect'
+import type { LocalTestContext, RootState } from './testUtils'
+import { localTest } from './testUtils'
+
+describe<LocalTestContext>('noopCheck', () => {
+  const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+  const identityFunction = vi.fn(<T>(state: T) => state)
+  const badSelector = createSelector(
+    [(state: RootState) => state],
+    identityFunction
+  )
+
+  afterEach(() => {
+    consoleSpy.mockClear()
+    identityFunction.mockClear()
+    badSelector.clearCache()
+    badSelector.memoizedResultFunc.clearCache()
+  })
+  afterAll(() => {
+    consoleSpy.mockRestore()
+  })
+  localTest(
+    'calls the result function twice, and warns to console if result is the same as argument',
+    ({ state }) => {
+      const goodSelector = createSelector(
+        [(state: RootState) => state],
+        state => state.todos
+      )
+
+      expect(goodSelector(state)).toBe(state.todos)
+
+      expect(consoleSpy).not.toHaveBeenCalled()
+
+      expect(badSelector(state)).toBe(state)
+
+      expect(identityFunction).toHaveBeenCalledTimes(2)
+
+      expect(consoleSpy).toHaveBeenCalled()
+    }
+  )
+
+  localTest('disables check if global setting is set to never', ({ state }) => {
+    setGlobalNoopCheck('never')
+
+    expect(badSelector(state)).toBe(state)
+
+    expect(identityFunction).toHaveBeenCalledTimes(1)
+
+    expect(consoleSpy).not.toHaveBeenCalled()
+
+    setGlobalNoopCheck('once')
+  })
+
+  localTest(
+    'disables check if specified in the selector options',
+    ({ state }) => {
+      const badSelector = createSelector(
+        [(state: RootState) => state],
+        identityFunction,
+        { noopCheck: 'never' }
+      )
+
+      expect(badSelector(state)).toBe(state)
+
+      expect(identityFunction).toHaveBeenCalledTimes(1)
+
+      expect(consoleSpy).not.toHaveBeenCalled()
+    }
+  )
+
+  localTest('disables check in production', ({ state }) => {
+    const originalEnv = process.env.NODE_ENV
+    process.env.NODE_ENV = 'production'
+
+    expect(badSelector(state)).toBe(state)
+
+    expect(identityFunction).toHaveBeenCalledTimes(1)
+
+    expect(consoleSpy).not.toHaveBeenCalled()
+
+    process.env.NODE_ENV = originalEnv
+  })
+
+  localTest('allows running the check only once', ({ state }) => {
+    const badSelector = createSelector(
+      [(state: RootState) => state],
+      identityFunction,
+      { noopCheck: 'once' }
+    )
+    expect(badSelector(state)).toBe(state)
+
+    expect(identityFunction).toHaveBeenCalledTimes(2)
+
+    expect(consoleSpy).toHaveBeenCalledOnce()
+
+    const newState = { ...state }
+
+    expect(badSelector(newState)).toBe(newState)
+
+    expect(identityFunction).toHaveBeenCalledTimes(3)
+
+    expect(consoleSpy).toHaveBeenCalledOnce()
+  })
+
+  localTest('uses the memoize provided', ({ state }) => {
+    // console.log(setupStore)
+    // const store = setupStore()
+    // const state = store.getState()
+    const badSelector = createSelector(
+      [(state: RootState) => state.todos],
+      identityFunction
+    )
+    expect(badSelector(state)).toBe(state.todos)
+
+    expect(identityFunction).toHaveBeenCalledTimes(2)
+
+    expect(consoleSpy).toHaveBeenCalledTimes(1)
+
+    const newState = { ...state }
+
+    expect(badSelector({ ...state })).not.toBe(state)
+
+    // expect(identityFunction).toHaveBeenCalledTimes(3)
+
+    expect(consoleSpy).toHaveBeenCalledTimes(1)
+  })
+})

test/reselect.spec.ts

@@ -214,7 +214,7 @@ describe('Basic selector behavior', () => {
     )
     expect(() => selector({ a: 1 })).toThrow('test error')
     expect(() => selector({ a: 1 })).toThrow('test error')
-    expect(called).toBe(2)
+    expect(called).toBe(3)
   })
 
   test('memoizes previous result before exception', () => {
@@ -232,7 +232,7 @@ describe('Basic selector behavior', () => {
     expect(selector(state1)).toBe(1)
     expect(() => selector(state2)).toThrow('test error')
     expect(selector(state1)).toBe(1)
-    expect(called).toBe(2)
+    expect(called).toBe(3)
   })
 })
 

test/testUtils.ts

@@ -1,13 +1,7 @@
 import type { PayloadAction } from '@reduxjs/toolkit'
 import { combineReducers, configureStore, createSlice } from '@reduxjs/toolkit'
 import { test } from 'vitest'
-import type {
-  AnyFunction,
-  OutputSelector,
-  Selector,
-  SelectorArray,
-  Simplify
-} from '../src/types'
+import type { AnyFunction, OutputSelector, Simplify } from '../src/types'
 
 interface Todo {
   id: number
@@ -415,20 +409,14 @@ export const localTest = test.extend<LocalTestContext>({
   state
 })
 
-export const resetSelector = <S extends OutputSelector>(
-  selector: S
-) => {
+export const resetSelector = <S extends OutputSelector>(selector: S) => {
   selector.clearCache()
   selector.resetRecomputations()
   selector.resetDependencyRecomputations()
   selector.memoizedResultFunc.clearCache()
 }
 
-export const logRecomputations = <
-  S extends OutputSelector
->(
-  selector: S
-) => {
+export const logRecomputations = <S extends OutputSelector>(selector: S) => {
   console.log(
     `${selector.name} result function recalculated:`,
     selector.recomputations(),

test/weakmapMemoize.spec.ts

@@ -193,7 +193,7 @@ describe('Basic selector behavior with weakMapMemoize', () => {
     )
     expect(() => selector({ a: 1 })).toThrow('test error')
     expect(() => selector({ a: 1 })).toThrow('test error')
-    expect(called).toBe(2)
+    expect(called).toBe(3)
   })
 
   test('memoizes previous result before exception', () => {
@@ -211,6 +211,6 @@ describe('Basic selector behavior with weakMapMemoize', () => {
     expect(selector(state1)).toBe(1)
     expect(() => selector(state2)).toThrow('test error')
     expect(selector(state1)).toBe(1)
-    expect(called).toBe(2)
+    expect(called).toBe(3)
   })
 })