From b86b2d61615239b38f982ba1ca49a3af920e13a6 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Fri, 1 Dec 2023 18:02:39 +0000 Subject: [PATCH 1/3] Add stack to dev check warnings --- src/devModeChecks/identityFunctionCheck.ts | 10 +++++++++- src/devModeChecks/inputStabilityCheck.ts | 11 +++++++++-- test/identityFunctionCheck.test.ts | 17 ++++++++++++++++- test/inputStabilityCheck.spec.ts | 3 ++- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/devModeChecks/identityFunctionCheck.ts b/src/devModeChecks/identityFunctionCheck.ts index dc4e3d450..65ce58d66 100644 --- a/src/devModeChecks/identityFunctionCheck.ts +++ b/src/devModeChecks/identityFunctionCheck.ts @@ -19,11 +19,19 @@ export const runIdentityFunctionCheck = (resultFunc: AnyFunction) => { // Do nothing } if (isInputSameAsOutput) { + let stack: string | undefined = undefined + try { + throw new Error() + } catch (e) { + // eslint-disable-next-line @typescript-eslint/no-extra-semi + ;({ stack } = e as Error) + } 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.' + '\nEnsure transformation logic is in the result function, and extraction logic is in the input selectors.', + { stack } ) } } diff --git a/src/devModeChecks/inputStabilityCheck.ts b/src/devModeChecks/inputStabilityCheck.ts index 4f62d418e..dab9e2780 100644 --- a/src/devModeChecks/inputStabilityCheck.ts +++ b/src/devModeChecks/inputStabilityCheck.ts @@ -31,7 +31,13 @@ export const runInputStabilityCheck = ( createAnEmptyObject.apply(null, inputSelectorResults) === createAnEmptyObject.apply(null, inputSelectorResultsCopy) if (!areInputSelectorResultsEqual) { - // do we want to log more information about the selector? + let stack: string | undefined = undefined + try { + throw new Error() + } catch (e) { + // eslint-disable-next-line @typescript-eslint/no-extra-semi + ;({ stack } = e as Error) + } console.warn( 'An input selector returned a different result when passed same arguments.' + '\nThis means your output selector will likely run more frequently than intended.' + @@ -40,7 +46,8 @@ export const runInputStabilityCheck = ( { arguments: inputSelectorArgs, firstInputs: inputSelectorResults, - secondInputs: inputSelectorResultsCopy + secondInputs: inputSelectorResultsCopy, + stack } ) } diff --git a/test/identityFunctionCheck.test.ts b/test/identityFunctionCheck.test.ts index d2f60967d..e54bf141a 100644 --- a/test/identityFunctionCheck.test.ts +++ b/test/identityFunctionCheck.test.ts @@ -2,7 +2,7 @@ import { createSelector, setGlobalDevModeChecks } from 'reselect' import type { LocalTestContext, RootState } from './testUtils' import { localTest } from './testUtils' -describe('identityFunctionCheck', () => { +describe('identityFunctionCheck', () => { const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) const identityFunction = vi.fn((state: T) => state) const badSelector = createSelector( @@ -39,6 +39,21 @@ describe('identityFunctionCheck', () => { } ) + localTest('includes stack with warning', ({ state }) => { + expect(badSelector(state)).toBe(state) + + expect(identityFunction).toHaveBeenCalledTimes(2) + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining( + 'The result function returned its own inputs without modification' + ), + { + stack: expect.any(String) + } + ) + }) + localTest('disables check if global setting is set to never', ({ state }) => { setGlobalDevModeChecks({ identityFunctionCheck: 'never' }) diff --git a/test/inputStabilityCheck.spec.ts b/test/inputStabilityCheck.spec.ts index f419f4387..bf29222f3 100644 --- a/test/inputStabilityCheck.spec.ts +++ b/test/inputStabilityCheck.spec.ts @@ -45,7 +45,8 @@ describe('inputStabilityCheck', () => { ]), secondInputs: expect.arrayContaining([ expect.objectContaining({ a: 1, b: 2 }) - ]) + ]), + stack: expect.any(String) }) ) }) From 75c2250f338dcba09c917e3e60c202a9c1a0dfb8 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Fri, 1 Dec 2023 18:05:13 +0000 Subject: [PATCH 2/3] fix assertion --- src/devModeChecks/identityFunctionCheck.ts | 2 +- src/devModeChecks/inputStabilityCheck.ts | 2 +- test/identityFunctionCheck.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/devModeChecks/identityFunctionCheck.ts b/src/devModeChecks/identityFunctionCheck.ts index 65ce58d66..6d8cfcdfa 100644 --- a/src/devModeChecks/identityFunctionCheck.ts +++ b/src/devModeChecks/identityFunctionCheck.ts @@ -23,7 +23,7 @@ export const runIdentityFunctionCheck = (resultFunc: AnyFunction) => { try { throw new Error() } catch (e) { - // eslint-disable-next-line @typescript-eslint/no-extra-semi + // eslint-disable-next-line @typescript-eslint/no-extra-semi, no-extra-semi ;({ stack } = e as Error) } console.warn( diff --git a/src/devModeChecks/inputStabilityCheck.ts b/src/devModeChecks/inputStabilityCheck.ts index dab9e2780..c2283e64f 100644 --- a/src/devModeChecks/inputStabilityCheck.ts +++ b/src/devModeChecks/inputStabilityCheck.ts @@ -35,7 +35,7 @@ export const runInputStabilityCheck = ( try { throw new Error() } catch (e) { - // eslint-disable-next-line @typescript-eslint/no-extra-semi + // eslint-disable-next-line @typescript-eslint/no-extra-semi, no-extra-semi ;({ stack } = e as Error) } console.warn( diff --git a/test/identityFunctionCheck.test.ts b/test/identityFunctionCheck.test.ts index e54bf141a..b35044523 100644 --- a/test/identityFunctionCheck.test.ts +++ b/test/identityFunctionCheck.test.ts @@ -42,7 +42,7 @@ describe('identityFunctionCheck', () => { localTest('includes stack with warning', ({ state }) => { expect(badSelector(state)).toBe(state) - expect(identityFunction).toHaveBeenCalledTimes(2) + expect(identityFunction).toHaveBeenCalledTimes(1) expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining( From 50a327ff88c562b5b2afd64ff4da4d2bb6c25cba Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Fri, 1 Dec 2023 18:34:34 +0000 Subject: [PATCH 3/3] completely recreate badSelector between tests so firstRun is reset --- package.json | 1 + test/identityFunctionCheck.test.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index b89f9faae..b396f5729 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "prepack": "yarn build", "bench": "vitest --run bench --mode production", "test": "node --expose-gc ./node_modules/vitest/dist/cli-wrapper.js run", + "test:watch": "node --expose-gc ./node_modules/vitest/dist/cli-wrapper.js watch", "test:cov": "vitest run --coverage", "type-check": "vitest --run typecheck", "type-check:trace": "vitest --run typecheck && tsc --noEmit -p typescript_test/tsconfig.json --generateTrace trace && npx @typescript/analyze-trace trace && rimraf trace", diff --git a/test/identityFunctionCheck.test.ts b/test/identityFunctionCheck.test.ts index b35044523..eee26f61f 100644 --- a/test/identityFunctionCheck.test.ts +++ b/test/identityFunctionCheck.test.ts @@ -5,7 +5,7 @@ import { localTest } from './testUtils' describe('identityFunctionCheck', () => { const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) const identityFunction = vi.fn((state: T) => state) - const badSelector = createSelector( + let badSelector = createSelector( [(state: RootState) => state], identityFunction ) @@ -13,8 +13,10 @@ describe('identityFunctionCheck', () => { afterEach(() => { consoleSpy.mockClear() identityFunction.mockClear() - badSelector.clearCache() - badSelector.memoizedResultFunc.clearCache() + badSelector = createSelector( + [(state: RootState) => state], + identityFunction + ) }) afterAll(() => { consoleSpy.mockRestore() @@ -42,7 +44,7 @@ describe('identityFunctionCheck', () => { localTest('includes stack with warning', ({ state }) => { expect(badSelector(state)).toBe(state) - expect(identityFunction).toHaveBeenCalledTimes(1) + expect(identityFunction).toHaveBeenCalledTimes(2) expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining(