Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning in development when a result function is x => x. #645

Merged
merged 24 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6cf53cb
Add initial implementation for `noopCheck`
aryaemami59 Nov 26, 2023
a8645a0
Disable no-op check for some unit tests
aryaemami59 Nov 26, 2023
703bf3a
Disable no-op check for performance tests
aryaemami59 Nov 26, 2023
1615733
Rename `StabilityCheckFrequency` to `DevModeCheckFrequency`
aryaemami59 Nov 26, 2023
cdc93c0
Rename `noopCheck` to `identityFunctionCheck`
aryaemami59 Nov 26, 2023
b25c9c9
Add JSDocs for `runIdentityFunctionCheck` utility function.
aryaemami59 Nov 26, 2023
ac4da4a
Update `README` to include `identityFunctionCheck`
aryaemami59 Nov 26, 2023
5a53b42
Add JSDocs for `setGlobalIdentityFunctionCheck`
aryaemami59 Nov 26, 2023
751daf3
Add JSDocs for `CreateSelectorOptions.identityFunctionCheck`
aryaemami59 Nov 26, 2023
611d833
Add `identityFunctionCheck` to v5 summary in `README`
aryaemami59 Nov 26, 2023
c2563d3
Fix copy-paste error in `README`
aryaemami59 Nov 26, 2023
45f036a
Fix JSDocs for `shouldRunDevModeCheck`
aryaemami59 Nov 26, 2023
c05bcc5
Fix issue with `firstRun`
aryaemami59 Nov 26, 2023
3a51095
Add `DevModeChecks`
aryaemami59 Nov 28, 2023
397f3d8
Update `README` with new dev-mode-check API
aryaemami59 Nov 28, 2023
dda015b
Fix `inputStabilityCheck` warning message.
aryaemami59 Nov 30, 2023
df84098
export `setGlobalDevModeChecks` in `index.ts`
aryaemami59 Nov 30, 2023
17718f3
Remove '@internal' imports
aryaemami59 Nov 30, 2023
4952ad4
Fix `devModeChecks` in type tests
aryaemami59 Nov 30, 2023
20a7d97
increase performance timer from 1000 to 1500
aryaemami59 Nov 30, 2023
bc7a188
Change package version as a workaround
aryaemami59 Nov 30, 2023
01792bc
Silence test logging
markerikson Dec 1, 2023
6f3bd23
Fix lockfile issue
markerikson Dec 1, 2023
ae0a921
Fix silly types import issue
markerikson Dec 1, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 84 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ Accepts either a `memoize` function and `...memoizeOptions` rest parameter, or s
| `options` | An options object containing the `memoize` function responsible for memoizing the `resultFunc` inside [`createSelector`] (e.g., `defaultMemoize` or `weakMapMemoize`). It also provides additional options for customizing memoization. While the `memoize` property is mandatory, the rest are optional. |
| `options.argsMemoize?` | The optional memoize function that is used to memoize the arguments passed into the [output selector] generated by [`createSelector`] (e.g., `defaultMemoize` or `weakMapMemoize`). <br /> **`Default`** `defaultMemoize` |
| `options.argsMemoizeOptions?` | Optional configuration options for the `argsMemoize` function. These options are passed to the `argsMemoize` function as the second argument. <br /> since 5.0.0 |
| `options.inputStabilityCheck?` | Overrides the global input stability check for the selector. Possible values are: <br /> `once` - Run only the first time the selector is called. <br /> `always` - Run every time the selector is called. <br /> `never` - Never run the input stability check. <br /> **`Default`** = `'once'` <br /> since 5.0.0 |
| `options.devModeChecks?` | Overrides the settings for the global development mode checks for the selector. <br /> since 5.0.0 |
| `options.memoize` | The memoize function that is used to memoize the `resultFunc` inside [`createSelector`] (e.g., `defaultMemoize` or `weakMapMemoize`). since 5.0.0 |
| `options.memoizeOptions?` | Optional configuration options for the `memoize` function. These options are passed to the `memoize` function as the second argument. <br /> since 5.0.0 |

Expand Down Expand Up @@ -1055,7 +1055,7 @@ const selectTodoIds = createSelectorAutotrack(

<a id="developmentonlychecks"></a>

### Development-Only Stability Checks
### Development-Only Checks

Reselect includes extra checks in development mode to help catch and warn about mistakes in selector behavior.

Expand All @@ -1079,7 +1079,7 @@ that will cause the selector to never memoize properly.
Since this is a common mistake, we've added a development mode check to catch this. By default, [`createSelector`] will now run the [input selectors] twice during the first call to the selector. If the result appears to be different for the same call, it will log a warning with the arguments and the two different sets of extracted input values.

```ts
type StabilityCheckFrequency = 'always' | 'once' | 'never'
type DevModeCheckFrequency = 'always' | 'once' | 'never'
```

| Possible Values | Description |
Expand All @@ -1093,43 +1093,112 @@ type StabilityCheckFrequency = 'always' | 'once' | 'never'

You can configure this behavior in two ways:

<a id="setinputstabilitycheckenabled"></a>
<a id="setglobaldevmodechecks"></a>

##### 1. Globally through `setInputStabilityCheckEnabled`:
##### 1. Globally through `setGlobalDevModeChecks`:

A `setInputStabilityCheckEnabled` function is exported from Reselect, which should be called with the desired setting.
A `setGlobalDevModeChecks` function is exported from Reselect, which should be called with the desired setting.

```ts
import { setInputStabilityCheckEnabled } from 'reselect'
import { setGlobalDevModeChecks } from 'reselect'

// Run only the first time the selector is called. (default)
setInputStabilityCheckEnabled('once')
setGlobalDevModeChecks({ inputStabilityCheck: 'once' })

// Run every time the selector is called.
setInputStabilityCheckEnabled('always')
setGlobalDevModeChecks({ inputStabilityCheck: 'always' })

// Never run the input stability check.
setInputStabilityCheckEnabled('never')
setGlobalDevModeChecks({ inputStabilityCheck: 'never' })
```

##### 2. Per selector by passing an `inputStabilityCheck` option directly to [`createSelector`]:

```ts
// Create a selector that double-checks the results of [`input selectors`][Input Selectors] every time it runs.
// Create a selector that double-checks the results of input selectors every time it runs.
const selectCompletedTodosLength = createSelector(
[
// This `input selector` will not be memoized properly since it always returns a new reference.
// ❌ Incorrect Use Case: This input selector will not be memoized properly since it always returns a new reference.
(state: RootState) =>
state.todos.filter(({ completed }) => completed === true)
],
completedTodos => completedTodos.length,
// Will override the global setting.
{ inputStabilityCheck: 'always' }
{ devModeChecks: { inputStabilityCheck: 'always' } }
)
```

> [!WARNING]
> This will override the global input stability check set by calling `setInputStabilityCheckEnabled`.
> This will override the global input stability check set by calling `setGlobalDevModeChecks`.

<a id="identityfunctioncheck"></a>

#### `identityFunctionCheck`

When working with Reselect, it's crucial to adhere to a fundamental philosophy regarding the separation of concerns between extraction and transformation logic.

- **Extraction Logic**: This refers to operations like `state => state.todos`, which should be placed in [input selectors]. Extraction logic is responsible for retrieving or 'selecting' data from a broader state or dataset.

- **Transformation Logic**: In contrast, transformation logic, such as `todos => todos.map(({ id }) => id)`, belongs in the [result function]. This is where you manipulate, format, or transform the data extracted by the input selectors.

Most importantly, effective memoization in Reselect hinges on following these guidelines. Memoization, only functions correctly when extraction and transformation logic are properly segregated. By keeping extraction logic in input selectors and transformation logic in the result function, Reselect can efficiently determine when to reuse cached results and when to recompute them. This not only enhances performance but also ensures the consistency and predictability of your selectors.

For memoization to work as intended, it's imperative to follow both guidelines. If either is disregarded, memoization will not function properly. Consider the following example for clarity:

```ts
// ❌ Incorrect Use Case: This will not memoize correctly, and does nothing useful!
const brokenSelector = createSelector(
aryaemami59 marked this conversation as resolved.
Show resolved Hide resolved
// ✔️ GOOD: Contains extraction logic.
[(state: RootState) => state.todos],
// ❌ BAD: Does not contain transformation logic.
todos => todos
)
```

```ts
type DevModeCheckFrequency = 'always' | 'once' | 'never'
```

| Possible Values | Description |
| :-------------- | :---------------------------------------------- |
| `once` | Run only the first time the selector is called. |
| `always` | Run every time the selector is called. |
| `never` | Never run the identity function check. |

> [!IMPORTANT]
> The identity function check is automatically disabled in production environments.

You can configure this behavior in two ways:

<a id="setGlobalDevModeChecks"></a>

##### 1. Globally through `setGlobalDevModeChecks`:

```ts
import { setGlobalDevModeChecks } from 'reselect'

// Run only the first time the selector is called. (default)
setGlobalDevModeChecks({ identityFunctionCheck: 'once' })

// Run every time the selector is called.
setGlobalDevModeChecks({ identityFunctionCheck: 'always' })

// Never run the identity function check.
setGlobalDevModeChecks({ identityFunctionCheck: 'never' })
```

##### 2. Per selector by passing an `identityFunctionCheck` option directly to [`createSelector`]:

```ts
// Create a selector that checks to see if the result function is an identity function.
const selectTodos = createSelector(
[(state: RootState) => state.todos],
// This result function does not contain any transformation logic.
todos => todos,
// Will override the global setting.
{ devModeChecks: { identityFunctionCheck: 'always' } }
)
```

<a id="outputselectorfields"></a>

Expand Down Expand Up @@ -1184,6 +1253,7 @@ Version 5.0.0 introduces several new features and improvements:

- Added `dependencyRecomputations` and `resetDependencyRecomputations` to the [output selector fields]. These additions provide greater control and insight over [input selectors], complementing the new `argsMemoize` API.
- Introduced `inputStabilityCheck`, a development tool that runs the [input selectors] twice using the same arguments and triggers a warning If they return differing results for the same call.
- Introduced `identityFunctionCheck`, a development tool that checks to see if the [result function] returns its own input.

These updates aim to enhance flexibility, performance, and developer experience. For detailed usage and examples, refer to the updated documentation sections for each feature.

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "reselect",
"version": "5.0.0-beta.1",
"version": "5.0.0-beta.2",
"description": "Selectors for Redux.",
"main": "./dist/cjs/reselect.cjs",
"module": "./dist/reselect.legacy-esm.js",
Expand Down
85 changes: 23 additions & 62 deletions src/createSelectorCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type {
SelectorArray,
SetRequired,
Simplify,
StabilityCheckFrequency,
UnknownMemoizer
} from './types'

Expand All @@ -22,8 +21,7 @@ import {
collectInputSelectorResults,
ensureIsArray,
getDependencies,
runStabilityCheck,
shouldRunInputStabilityCheck
getDevModeChecksExecutionInfo
} from './utils'

/**
Expand Down Expand Up @@ -143,50 +141,6 @@ export interface CreateSelectorFunction<
InterruptRecursion
}

let globalStabilityCheck: StabilityCheckFrequency = 'once'

/**
* In development mode, an extra check is conducted on your input selectors.
* It runs your input selectors an extra time with the same arguments, and
* warns in the console if they return a different result _(based on your `memoize` method)_.
*
* This function allows you to override this setting for all of your selectors.
*
* **Note**: This setting can still be overridden per selector inside `createSelector`'s `options` object.
* See {@link https://github.com/reduxjs/reselect#2-per-selector-by-passing-an-inputstabilitycheck-option-directly-to-createselector per-selector-configuration}
* and {@linkcode CreateSelectorOptions.inputStabilityCheck inputStabilityCheck} for more details.
*
* _The input stability check does not run in production builds._
*
* @param inputStabilityCheckFrequency - How often the `inputStabilityCheck` should run for all selectors.
*
* @example
* ```ts
* import { setInputStabilityCheckEnabled } from 'reselect'
import { assert } from './autotrackMemoize/utils';
import { OutputSelectorFields, Mapped } from './types';
*
* // Run only the first time the selector is called. (default)
* setInputStabilityCheckEnabled('once')
*
* // Run every time the selector is called.
* setInputStabilityCheckEnabled('always')
*
* // Never run the input stability check.
* setInputStabilityCheckEnabled('never')
* ```
* @see {@link https://github.com/reduxjs/reselect#debugging-tools debugging-tools}
* @see {@link https://github.com/reduxjs/reselect#1-globally-through-setinputstabilitycheckenabled global-configuration}
*
* @since 5.0.0
* @public
*/
export function setInputStabilityCheckEnabled(
inputStabilityCheckFrequency: StabilityCheckFrequency
) {
globalStabilityCheck = inputStabilityCheckFrequency
}

/**
* Creates a selector creator function with the specified memoization function and options for customizing memoization behavior.
*
Expand Down Expand Up @@ -374,7 +328,7 @@ export function createSelectorCreator<
memoizeOptions = [],
argsMemoize = weakMapMemoize,
argsMemoizeOptions = [],
inputStabilityCheck = globalStabilityCheck
devModeChecks = {}
} = combinedOptions

// Simplifying assumption: it's unlikely that the first options arg of the provided memoizer
Expand Down Expand Up @@ -408,21 +362,28 @@ export function createSelectorCreator<
arguments
)

if (
process.env.NODE_ENV !== 'production' &&
shouldRunInputStabilityCheck(inputStabilityCheck, firstRun)
) {
// make a second copy of the params, to check if we got the same results
const inputSelectorResultsCopy = collectInputSelectorResults(
dependencies,
arguments
)
if (process.env.NODE_ENV !== 'production') {
const { identityFunctionCheck, inputStabilityCheck } =
getDevModeChecksExecutionInfo(firstRun, devModeChecks)
if (identityFunctionCheck.shouldRun) {
identityFunctionCheck.run(
resultFunc as Combiner<InputSelectors, Result>
)
}

if (inputStabilityCheck.shouldRun) {
// make a second copy of the params, to check if we got the same results
const inputSelectorResultsCopy = collectInputSelectorResults(
dependencies,
arguments
)

runStabilityCheck(
{ inputSelectorResults, inputSelectorResultsCopy },
{ memoize, memoizeOptions: finalMemoizeOptions },
arguments
)
inputStabilityCheck.run(
{ inputSelectorResults, inputSelectorResultsCopy },
{ memoize, memoizeOptions: finalMemoizeOptions },
arguments
)
}

if (firstRun) firstRun = false
}
Expand Down
29 changes: 29 additions & 0 deletions src/devModeChecks/identityFunctionCheck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { AnyFunction } from '../types'

/**
* Runs a check to determine if the given result function behaves as an
* identity function. An identity function is one that returns its
* input unchanged, for example, `x => x`. This check helps ensure
* efficient memoization and prevent unnecessary re-renders by encouraging
* proper use of transformation logic in result functions and
* extraction logic in input selectors.
*
* @param resultFunc - The result function to be checked.
*/
export const runIdentityFunctionCheck = (resultFunc: AnyFunction) => {
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.'
)
}
}
47 changes: 47 additions & 0 deletions src/devModeChecks/inputStabilityCheck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import type { CreateSelectorOptions, UnknownMemoizer } from '../types'

/**
* Runs a stability check to ensure the input selector results remain stable
* when provided with the same arguments. This function is designed to detect
* changes in the output of input selectors, which can impact the performance of memoized selectors.
*
* @param inputSelectorResultsObject - An object containing two arrays: `inputSelectorResults` and `inputSelectorResultsCopy`, representing the results of input selectors.
* @param options - Options object consisting of a `memoize` function and a `memoizeOptions` object.
* @param inputSelectorArgs - List of arguments being passed to the input selectors.
*/
export const runInputStabilityCheck = (
inputSelectorResultsObject: {
inputSelectorResults: unknown[]
inputSelectorResultsCopy: unknown[]
},
options: Required<
Pick<
CreateSelectorOptions<UnknownMemoizer, UnknownMemoizer>,
'memoize' | 'memoizeOptions'
>
>,
inputSelectorArgs: unknown[] | IArguments
) => {
const { memoize, memoizeOptions } = options
const { inputSelectorResults, inputSelectorResultsCopy } =
inputSelectorResultsObject
const createAnEmptyObject = memoize(() => ({}), ...memoizeOptions)
// if the memoize method thinks the parameters are equal, these *should* be the same reference
const areInputSelectorResultsEqual =
createAnEmptyObject.apply(null, inputSelectorResults) ===
createAnEmptyObject.apply(null, inputSelectorResultsCopy)
if (!areInputSelectorResultsEqual) {
// do we want to log more information about the selector?
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.' +
'\nAvoid returning a new reference inside your input selector, e.g.' +
'\n`createSelector([state => state.todos.map(todo => todo.id)], todoIds => todoIds.length)`',
{
arguments: inputSelectorArgs,
firstInputs: inputSelectorResults,
secondInputs: inputSelectorResultsCopy
}
)
}
}
Loading
Loading