From 9aff7bb93630553faad104895ab7b5e1d102e114 Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Wed, 1 Jun 2022 12:05:03 +0100 Subject: [PATCH 1/2] Check initial state is draftable before using immer to freeze it. --- packages/toolkit/src/createReducer.ts | 10 +++++++--- packages/toolkit/src/tests/createReducer.test.ts | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/toolkit/src/createReducer.ts b/packages/toolkit/src/createReducer.ts index 06e48467bb..cae2ab5549 100644 --- a/packages/toolkit/src/createReducer.ts +++ b/packages/toolkit/src/createReducer.ts @@ -75,6 +75,10 @@ function isStateFunction(x: unknown): x is () => S { return typeof x === 'function' } +function freezeDraftable(val: T) { + return isDraftable(val) ? createNextState(val, () => {}) : val +} + export type ReducerWithInitialState> = Reducer & { getInitialState: () => S } @@ -223,12 +227,12 @@ export function createReducer>( ? executeReducerBuilderCallback(mapOrBuilderCallback) : [mapOrBuilderCallback, actionMatchers, defaultCaseReducer] - // Ensure the initial state gets frozen either way + // Ensure the initial state gets frozen either way (if draftable) let getInitialState: () => S if (isStateFunction(initialState)) { - getInitialState = () => createNextState(initialState(), () => {}) + getInitialState = () => freezeDraftable(initialState()) } else { - const frozenInitialState = createNextState(initialState, () => {}) + const frozenInitialState = freezeDraftable(initialState) getInitialState = () => frozenInitialState } diff --git a/packages/toolkit/src/tests/createReducer.test.ts b/packages/toolkit/src/tests/createReducer.test.ts index 97985991ed..2356d20918 100644 --- a/packages/toolkit/src/tests/createReducer.test.ts +++ b/packages/toolkit/src/tests/createReducer.test.ts @@ -106,6 +106,9 @@ describe('createReducer', () => { /Cannot assign to read only property/ ) }) + test('does not throw error if initial state is not draftable', () => { + expect(() => createReducer(new URLSearchParams(), {})).not.toThrowError() + }) }) describe('given pure reducers with immutable updates', () => { From 206a1b0a4b64eb206d16efc2e658e911c58287ce Mon Sep 17 00:00:00 2001 From: Ben Durrant Date: Wed, 1 Jun 2022 12:28:44 +0100 Subject: [PATCH 2/2] Move freezeDraftable to utils and use in createSlice too. --- packages/toolkit/src/createReducer.ts | 5 +---- packages/toolkit/src/createSlice.ts | 20 +++++++++---------- .../toolkit/src/tests/createSlice.test.ts | 20 +++++++++++++++++++ packages/toolkit/src/utils.ts | 5 +++++ 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/toolkit/src/createReducer.ts b/packages/toolkit/src/createReducer.ts index cae2ab5549..ca26edfa95 100644 --- a/packages/toolkit/src/createReducer.ts +++ b/packages/toolkit/src/createReducer.ts @@ -4,6 +4,7 @@ import type { AnyAction, Action, Reducer } from 'redux' import type { ActionReducerMapBuilder } from './mapBuilders' import { executeReducerBuilderCallback } from './mapBuilders' import type { NoInfer } from './tsHelpers' +import { freezeDraftable } from './utils' /** * Defines a mapping from action types to corresponding action object shapes. @@ -75,10 +76,6 @@ function isStateFunction(x: unknown): x is () => S { return typeof x === 'function' } -function freezeDraftable(val: T) { - return isDraftable(val) ? createNextState(val, () => {}) : val -} - export type ReducerWithInitialState> = Reducer & { getInitialState: () => S } diff --git a/packages/toolkit/src/createSlice.ts b/packages/toolkit/src/createSlice.ts index 5e120ae4c4..bbebc09bdb 100644 --- a/packages/toolkit/src/createSlice.ts +++ b/packages/toolkit/src/createSlice.ts @@ -17,6 +17,7 @@ import { createReducer, NotFunction } from './createReducer' import type { ActionReducerMapBuilder } from './mapBuilders' import { executeReducerBuilderCallback } from './mapBuilders' import type { NoInfer } from './tsHelpers' +import { freezeDraftable } from './utils' /** * An action creator attached to a slice. @@ -226,16 +227,15 @@ type SliceDefinedCaseReducers> = { export type ValidateSliceCaseReducers< S, ACR extends SliceCaseReducers -> = ACR & - { - [T in keyof ACR]: ACR[T] extends { - reducer(s: S, action?: infer A): any - } - ? { - prepare(...a: never[]): Omit - } - : {} +> = ACR & { + [T in keyof ACR]: ACR[T] extends { + reducer(s: S, action?: infer A): any } + ? { + prepare(...a: never[]): Omit + } + : {} +} function getType(slice: string, actionKey: string): string { return `${slice}/${actionKey}` @@ -265,7 +265,7 @@ export function createSlice< const initialState = typeof options.initialState == 'function' ? options.initialState - : createNextState(options.initialState, () => {}) + : freezeDraftable(options.initialState) const reducers = options.reducers || {} diff --git a/packages/toolkit/src/tests/createSlice.test.ts b/packages/toolkit/src/tests/createSlice.test.ts index 3aee1309d3..5b8ba63365 100644 --- a/packages/toolkit/src/tests/createSlice.test.ts +++ b/packages/toolkit/src/tests/createSlice.test.ts @@ -74,6 +74,16 @@ describe('createSlice', () => { expect(slice.getInitialState()).toBe(initialState) }) + + it('should allow non-draftable initial state', () => { + expect(() => + createSlice({ + name: 'params', + initialState: new URLSearchParams(), + reducers: {}, + }) + ).not.toThrowError() + }) }) describe('when initialState is a function', () => { @@ -105,6 +115,16 @@ describe('createSlice', () => { expect(slice.getInitialState()).toBe(42) }) + + it('should allow non-draftable initial state', () => { + expect(() => + createSlice({ + name: 'params', + initialState: () => new URLSearchParams(), + reducers: {}, + }) + ).not.toThrowError() + }) }) describe('when mutating state object', () => { diff --git a/packages/toolkit/src/utils.ts b/packages/toolkit/src/utils.ts index 5a3c77e1da..c16eb8355a 100644 --- a/packages/toolkit/src/utils.ts +++ b/packages/toolkit/src/utils.ts @@ -1,3 +1,4 @@ +import createNextState, { isDraftable } from 'immer' import type { Middleware } from 'redux' export function getTimeMeasureUtils(maxDelay: number, fnName: string) { @@ -64,3 +65,7 @@ export class MiddlewareArray< return new MiddlewareArray(...arr.concat(this)) } } + +export function freezeDraftable(val: T) { + return isDraftable(val) ? createNextState(val, () => {}) : val +}