diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 483926fba1..58ad0cb00f 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -274,7 +274,7 @@ export interface EntityStateAdapter { } // @public (undocumented) -export function findNonSerializableValue(value: unknown, path?: ReadonlyArray, isSerializable?: (value: unknown) => boolean, getEntries?: (value: unknown) => [string, any][], ignoredPaths?: string[]): NonSerializableValue | false; +export function findNonSerializableValue(value: unknown, path?: string, isSerializable?: (value: unknown) => boolean, getEntries?: (value: unknown) => [string, any][], ignoredPaths?: string[]): NonSerializableValue | false; export { freeze } @@ -419,6 +419,7 @@ export interface SerializableStateInvariantMiddlewareOptions { ignoredActionPaths?: string[]; ignoredActions?: string[]; ignoredPaths?: string[]; + ignoreState?: boolean; isSerializable?: (value: any) => boolean; warnAfter?: number; } diff --git a/src/immutablePerfBenchmarks.ts b/src/immutablePerfBenchmarks.ts new file mode 100644 index 0000000000..d535bf0246 --- /dev/null +++ b/src/immutablePerfBenchmarks.ts @@ -0,0 +1,256 @@ +// import Benchmark from 'benchmark' +import { Store, MiddlewareAPI, Dispatch } from 'redux' +import faker from 'faker' + +import { configureStore } from './configureStore' +import { createSlice } from './createSlice' + +import { + createImmutableStateInvariantMiddleware, + tm2, + ImmutableStateInvariantMiddlewareOptions +} from './immutableStateInvariantMiddleware' + +export class TaskInfo { + private _taskName: string + private _percentage: string | undefined + private _timeMillis: number + + constructor(taskName: string, timeMillis: number) { + this._taskName = taskName + this._timeMillis = timeMillis + } + + get taskName(): string { + return this._taskName + } + + get timeMills(): number { + return this._timeMillis + } + + get percentage(): string | undefined { + return this._percentage + } + + calculatePercentage(totalTimeMillis: number): string { + this._percentage = ((this._timeMillis * 100) / totalTimeMillis).toFixed(2) + return this._percentage + } +} + +export class StopWatch { + public static NoTaskMessage = 'No task info kept' + + private id: string + private currentTaskName: string | null = null + private startTimeMillis = 0 + private totalTimeMillis = 0 + private taskList: Array = [] + + constructor(id = '') { + this.id = id + } + + /** + * start a task + */ + start(taskName = ''): void { + this.currentTaskName !== null && + this.throwError("Can't start StopWatch: it's already running") + this.currentTaskName = taskName + this.startTimeMillis = Date.now() + } + + /** + * stop the current task + */ + stop(): void { + this.currentTaskName === null && + this.throwError("Can't stop StopWatch: it's not running") + const lastTime: number = Date.now() - this.startTimeMillis + this.totalTimeMillis += lastTime + const lastTaskInfo = new TaskInfo(this.currentTaskName!, lastTime) + this.taskList.push(lastTaskInfo) + this.currentTaskName = null + } + + /** + * Return a string with a table describing all tasks performed. + */ + prettyPrint(): string { + const output: Array = [this.shortSummary()] + if (this.taskList.length) { + output.push('------------------------------------------') + output.push('ms \t\t % \t\t Task name') + output.push('------------------------------------------') + this.taskList.forEach((task: TaskInfo) => { + let percentage = '0' + try { + percentage = task.calculatePercentage(this.totalTimeMillis) + } catch (e) {} + output.push( + `${task.timeMills} \t\t ${percentage} \t\t ${task.taskName}` + ) + }) + } else { + output.push(StopWatch.NoTaskMessage) + } + const outputString = output.join('\n') + + console.info(outputString) + return outputString + } + + /** + * Return a task matching the given name + */ + getTask(taskName: string): TaskInfo | undefined { + const task = this.taskList.find(task => task.taskName === taskName) + if (task) { + task.calculatePercentage(this.totalTimeMillis) + } + + return task + } + + /** + * Return the total running time in milliseconds + */ + getTotalTime(): number { + return this.totalTimeMillis + } + + /** + * Return a short description of the total running time. + */ + shortSummary(): string { + return `StopWatch '${this.id}' running time (millis) = ${this.totalTimeMillis}` + } + + /** + * Return whether the stop watch is currently running + */ + isRunning(): boolean { + return this.currentTaskName !== null + } + + /** + * Return the number of tasks timed. + */ + getTaskCount(): number { + return this.taskList.length + } + + private throwError(msg: string): never { + throw new Error(msg) + } +} + +/* +let state: any +const getState: Store['getState'] = () => state + +function middleware(options: ImmutableStateInvariantMiddlewareOptions = {}) { + return createImmutableStateInvariantMiddleware(options)({ + getState + } as MiddlewareAPI) +} + +const next: Dispatch = action => action +*/ + +function createSliceData() { + const people = Array.from({ length: 10000 }).map( + () => faker.helpers.userCard() as any + ) + people.forEach(person => { + person.vehicles = Array.from({ length: 2 }).map(() => + faker.vehicle.vehicle() + ) + }) + + return people +} + +const state: any = { + a: createSliceData(), + b: createSliceData(), + c: createSliceData() +} + +// debugger +// let q = 42 + +const dummySlice = createSlice({ + name: 'dummy', + initialState: state, + reducers: {} +}) + +const originalStore = configureStore({ + reducer: dummySlice.reducer, + middleware: gdm => + gdm({ + // serializableCheck: false + }) +}) + +function runOriginal() { + // const dispatch = middleware()(next) + // dispatch({ type: 'SOME_ACTION' }) + originalStore.dispatch({ type: 'SOME_ACTION' }) +} + +const queuedStore = configureStore({ + reducer: dummySlice.reducer, + middleware: gdm => + gdm({ + // serializableCheck: false, + immutableCheck: { + trackFunction: tm2 + } + }) +}) + +function runQueued() { + queuedStore.dispatch({ type: 'SOME_ACTION' }) +} +/* +const suite = new Benchmark.Suite('abcd', { + setup() { + state = { + a: createSliceData(), + b: createSliceData(), + c: createSliceData() + } + } +}) + +suite + .add('Original', ) + .add('Queued', ) + .on('cycle', function(event: any) { + console.log(String(event.target)) + }) + .on('complete', function(this: any) { + console.log('Fastest is ' + this.filter('fastest').map('name')) + }) + .run({}) +*/ + +const stopwatch = new StopWatch() + +stopwatch.start('Original') +runOriginal() +stopwatch.stop() + +stopwatch.start('Queued') +runQueued() +stopwatch.stop() + +// debugger + +stopwatch.prettyPrint() + +// let z = q diff --git a/src/immutableStateInvariantMiddleware.test.ts b/src/immutableStateInvariantMiddleware.test.ts index ab89d71e22..0e0aae0269 100644 --- a/src/immutableStateInvariantMiddleware.test.ts +++ b/src/immutableStateInvariantMiddleware.test.ts @@ -184,7 +184,7 @@ describe('trackForMutations', () => { expect(tracker.detectMutations()).toEqual({ wasMutated: true, - path: spec.path + path: spec.path.join('.') }) }) } diff --git a/src/immutableStateInvariantMiddleware.ts b/src/immutableStateInvariantMiddleware.ts index ba966a846e..843f24fa99 100644 --- a/src/immutableStateInvariantMiddleware.ts +++ b/src/immutableStateInvariantMiddleware.ts @@ -68,7 +68,10 @@ function getSerialize( */ export function isImmutableDefault(value: unknown): boolean { return ( - typeof value !== 'object' || value === null || typeof value === 'undefined' + typeof value !== 'object' || + value === null || + typeof value === 'undefined' || + Object.isFrozen(value) ) } @@ -94,7 +97,7 @@ function trackProperties( isImmutable: IsImmutableFunc, ignorePaths: IgnorePaths = [], obj: Record, - path: string[] = [] + path: string = '' ) { const tracked: Partial = { value: obj } @@ -102,11 +105,8 @@ function trackProperties( tracked.children = {} for (const key in obj) { - const childPath = path.concat(key) - if ( - ignorePaths.length && - ignorePaths.indexOf(childPath.join('.')) !== -1 - ) { + const childPath = path ? path + '.' + key : key + if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) { continue } @@ -129,8 +129,8 @@ function detectMutations( trackedProperty: TrackedProperty, obj: any, sameParentRef: boolean = false, - path: string[] = [] -): { wasMutated: boolean; path?: string[] } { + path: string = '' +): { wasMutated: boolean; path?: string } { const prevObj = trackedProperty ? trackedProperty.value : undefined const sameRef = prevObj === obj @@ -145,18 +145,16 @@ function detectMutations( // Gather all keys from prev (tracked) and after objs const keysToDetect: Record = {} - Object.keys(trackedProperty.children).forEach(key => { + for (let key in trackedProperty.children) { keysToDetect[key] = true - }) - Object.keys(obj).forEach(key => { + } + for (let key in obj) { keysToDetect[key] = true - }) + } - const keys = Object.keys(keysToDetect) - for (let i = 0; i < keys.length; i++) { - const key = keys[i] - const childPath = path.concat(key) - if (ignorePaths.length && ignorePaths.indexOf(childPath.join('.')) !== -1) { + for (let key in keysToDetect) { + const childPath = path ? path + '.' + key : key + if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) { continue } @@ -251,11 +249,8 @@ export function createImmutableStateInvariantMiddleware( invariant( !result.wasMutated, - `A state mutation was detected between dispatches, in the path '${( - result.path || [] - ).join( - '.' - )}'. This may cause incorrect behavior. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)` + `A state mutation was detected between dispatches, in the path '${result.path || + ''}'. This may cause incorrect behavior. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)` ) }) @@ -271,13 +266,10 @@ export function createImmutableStateInvariantMiddleware( result.wasMutated && invariant( !result.wasMutated, - `A state mutation was detected inside a dispatch, in the path: ${( - result.path || [] - ).join( - '.' - )}. Take a look at the reducer(s) handling the action ${stringify( + `A state mutation was detected inside a dispatch, in the path: ${result.path || + ''}. Take a look at the reducer(s) handling the action ${stringify( action - )}. (https://redux.js.org/troubleshooting#never-mutate-reducer-arguments)` + )}. (https://redux.js.org/style-guide/style-guide#do-not-mutate-state)` ) }) diff --git a/src/serializableStateInvariantMiddleware.test.ts b/src/serializableStateInvariantMiddleware.test.ts index 6e769776e5..c3f1644cea 100644 --- a/src/serializableStateInvariantMiddleware.test.ts +++ b/src/serializableStateInvariantMiddleware.test.ts @@ -443,6 +443,21 @@ describe('serializableStateInvariantMiddleware', () => { `) }) + it('allows ignoring state entirely', () => { + const badValue = new Map() + const reducer = () => badValue + configureStore({ + reducer, + middleware: [ + createSerializableStateInvariantMiddleware({ + ignoreState: true + }) + ] + }).dispatch({ type: 'test' }) + + expect(getLog().log).toMatchInlineSnapshot(`""`) + }) + it('Should print a warning if execution takes too long', () => { const reducer: Reducer = (state = 42, action) => { return state diff --git a/src/serializableStateInvariantMiddleware.ts b/src/serializableStateInvariantMiddleware.ts index fbd2d84203..dd076b17ce 100644 --- a/src/serializableStateInvariantMiddleware.ts +++ b/src/serializableStateInvariantMiddleware.ts @@ -12,12 +12,13 @@ import { getTimeMeasureUtils } from './utils' * @public */ export function isPlain(val: any) { + const type = typeof val return ( - typeof val === 'undefined' || + type === 'undefined' || val === null || - typeof val === 'string' || - typeof val === 'boolean' || - typeof val === 'number' || + type === 'string' || + type === 'boolean' || + type === 'number' || Array.isArray(val) || isPlainObject(val) ) @@ -33,7 +34,7 @@ interface NonSerializableValue { */ export function findNonSerializableValue( value: unknown, - path: ReadonlyArray = [], + path: string = '', isSerializable: (value: unknown) => boolean = isPlain, getEntries?: (value: unknown) => [string, any][], ignoredPaths: string[] = [] @@ -42,7 +43,7 @@ export function findNonSerializableValue( if (!isSerializable(value)) { return { - keyPath: path.join('.') || '', + keyPath: path || '', value: value } } @@ -55,16 +56,16 @@ export function findNonSerializableValue( const hasIgnoredPaths = ignoredPaths.length > 0 - for (const [property, nestedValue] of entries) { - const nestedPath = path.concat(property) + for (const [key, nestedValue] of entries) { + const nestedPath = path ? path + '.' + key : key // path.concat(property) - if (hasIgnoredPaths && ignoredPaths.indexOf(nestedPath.join('.')) >= 0) { + if (hasIgnoredPaths && ignoredPaths.indexOf(nestedPath) >= 0) { continue } if (!isSerializable(nestedValue)) { return { - keyPath: nestedPath.join('.'), + keyPath: nestedPath, value: nestedValue } } @@ -129,6 +130,11 @@ export interface SerializableStateInvariantMiddlewareOptions { * Defaults to 32ms. */ warnAfter?: number + + /** + * Opt out of checking state, but continue checking actions + */ + ignoreState?: boolean } /** @@ -152,7 +158,8 @@ export function createSerializableStateInvariantMiddleware( ignoredActions = [], ignoredActionPaths = ['meta.arg'], ignoredPaths = [], - warnAfter = 32 + warnAfter = 32, + ignoreState = false } = options return storeAPI => next => action => { @@ -167,7 +174,7 @@ export function createSerializableStateInvariantMiddleware( measureUtils.measureTime(() => { const foundActionNonSerializableValue = findNonSerializableValue( action, - [], + '', isSerializable, getEntries, ignoredActionPaths @@ -189,31 +196,33 @@ export function createSerializableStateInvariantMiddleware( const result = next(action) - measureUtils.measureTime(() => { - const state = storeAPI.getState() + if (!ignoreState) { + measureUtils.measureTime(() => { + const state = storeAPI.getState() - const foundStateNonSerializableValue = findNonSerializableValue( - state, - [], - isSerializable, - getEntries, - ignoredPaths - ) + const foundStateNonSerializableValue = findNonSerializableValue( + state, + '', + isSerializable, + getEntries, + ignoredPaths + ) - if (foundStateNonSerializableValue) { - const { keyPath, value } = foundStateNonSerializableValue + if (foundStateNonSerializableValue) { + const { keyPath, value } = foundStateNonSerializableValue - console.error( - `A non-serializable value was detected in the state, in the path: \`${keyPath}\`. Value:`, - value, - ` + console.error( + `A non-serializable value was detected in the state, in the path: \`${keyPath}\`. Value:`, + value, + ` Take a look at the reducer(s) handling this action type: ${action.type}. (See https://redux.js.org/faq/organizing-state#can-i-put-functions-promises-or-other-non-serializable-items-in-my-store-state)` - ) - } - }) + ) + } + }) - measureUtils.warnIfExceeded() + measureUtils.warnIfExceeded() + } return result } diff --git a/tsconfig.perf.json b/tsconfig.perf.json new file mode 100644 index 0000000000..92b3b28835 --- /dev/null +++ b/tsconfig.perf.json @@ -0,0 +1,17 @@ +{ + "compilerOptions": { + "allowSyntheticDefaultImports": true, + "esModuleInterop": true, + "declaration": true, + "module": "CommonJS", + "moduleResolution": "node", + "noUnusedLocals": false, + "noUnusedParameters": false, + "outDir": "dist", + "strict": true, + "target": "es2018", + "sourceMap": true + }, + "include": ["src"], + "exclude": ["src/*.test.ts"] +}