diff --git a/README.md b/README.md index ec63d76..f0cbbb5 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ In this basic example, the changes to the draft are 'mutative' within the draft > Forbid accessing non-draftable values in strict mode(unless using [unsafe()](#unsafe)). - > It is recommended to enable `strict` in development mode and disable `strict` in production mode. This will ensure safe returns and also keep good performance in the production build. If the return value is mixed drafts or `undefined`, then use [safeReturn()](#safereturn). + > It is recommended to enable `strict` in development mode and disable `strict` in production mode. This will ensure safe explicit returns and also keep good performance in the production build. If the value that does not mix any current draft or is `undefined` is returned, then use [rawReturn()](#rawreturn). - enablePatches - `boolean | { pathAsArray?: boolean; arrayLengthAssignment?: boolean; }`, the default is false. @@ -302,19 +302,33 @@ expect(isDraftable(baseState.list)).toBeTruthy(); > You can set a mark to determine if the value is draftable, and the mark function should be the same as passing in `create()` mark option. -### `safeReturn()` +### `rawReturn()` -It is used as a safe return value to ensure that this value replaces the finalized draft value or use it to return `undefined` explicitly. +It is used as a raw return value to ensure that this value replaces the finalized draft value or use it to return `undefined` explicitly. ```ts const baseState = { id: 'test' }; const state = create(baseState as { id: string } | undefined, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); expect(state).toBe(undefined); ``` -> You don't need to use `safeReturn()` when the return value doesn't have any draft. +> You don't need to use `rawReturn()` when the return value have any draft. + +```ts +const baseState = { a: 1, b: { c: 1 } }; +const state = create(baseState, (draft) => { + if (draft.b.c === 1) { + return { + ...draft, + a: 2, + }; + } +}); +expect(state).toEqual({ a: 2, b: { c: 1 } }); +expect(isDraft(state.b)).toBeFalsy(); +``` [View more API docs](./docs/README.md). @@ -440,10 +454,10 @@ const nextState = produce(baseState, (draft) => { Use Mutative ```ts -import { create, safeReturn } from 'mutative'; +import { create, rawReturn } from 'mutative'; const nextState = create(baseState, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); ``` diff --git a/src/constant.ts b/src/constant.ts index 4d37dc8..abc1b23 100644 --- a/src/constant.ts +++ b/src/constant.ts @@ -1,5 +1,6 @@ // Don't use `Symbol()` just for 3rd party access the draft export const PROXY_DRAFT = Symbol.for('__MUTATIVE_PROXY_DRAFT__'); +export const RAW_RETURN_SYMBOL = Symbol('__MUTATIVE_RAW_RETURN_SYMBOL__'); export const iteratorSymbol: typeof Symbol.iterator = Symbol.iterator; diff --git a/src/create.ts b/src/create.ts index 6bf75cf..07a11a1 100644 --- a/src/create.ts +++ b/src/create.ts @@ -14,7 +14,7 @@ import { revokeProxy, } from './utils'; import { current, handleReturnValue } from './current'; -import { safeReturnValue } from './safeReturn'; +import { RAW_RETURN_SYMBOL } from './constant'; /** * `create(baseState, callback, options)` to create the next state @@ -108,7 +108,6 @@ function create(arg0: any, arg1: any, arg2?: any): any { strict, enablePatches, }; - safeReturnValue.length = 0; if ( !isDraftable(state, _options) && typeof state === 'object' && @@ -146,16 +145,17 @@ function create(arg0: any, arg1: any, arg2?: any): any { `Either the value is returned as a new non-draft value, or only the draft is modified without returning any value.` ); } - if (safeReturnValue.length) { - const _value = safeReturnValue.pop(); - if (typeof value === 'object' && value !== null) { - handleReturnValue(value); + const rawReturnValue = value?.[RAW_RETURN_SYMBOL] as [any] | undefined; + if (rawReturnValue) { + const _value = rawReturnValue[0]; + if (_options.strict && typeof value === 'object' && value !== null) { + handleReturnValue(proxyDraft, value, true); } return finalize([_value]); } if (value !== undefined) { - if (_options.strict && typeof value === 'object' && value !== null) { - handleReturnValue(value, true); + if (typeof value === 'object' && value !== null) { + handleReturnValue(proxyDraft, value); } return finalize([value]); } diff --git a/src/current.ts b/src/current.ts index acc1bec..033d3dd 100644 --- a/src/current.ts +++ b/src/current.ts @@ -1,4 +1,4 @@ -import { DraftType } from './interface'; +import { DraftType, ProxyDraft } from './interface'; import { forEach, get, @@ -11,13 +11,20 @@ import { shallowCopy, } from './utils'; -export function handleReturnValue(value: T, warning = false) { +export function handleReturnValue( + rootDraft: ProxyDraft | undefined, + value: T, + warning = false +) { + let isContainDraft = false; forEach(value, (key, item, source) => { const proxyDraft = getProxyDraft(item); - if (proxyDraft) { + // just handle the draft which is created by the same rootDraft + if (proxyDraft && proxyDraft?.finalities === rootDraft?.finalities) { + isContainDraft = true; if (__DEV__ && warning) { console.warn( - `The return value contains drafts, please use safeReturn() to wrap the return value.` + `The return value contains drafts, please don't use 'rawReturn()' to wrap the return value.` ); } const currentValue = proxyDraft.original; @@ -31,9 +38,14 @@ export function handleReturnValue(value: T, warning = false) { set(source, key, currentValue); } } else if (typeof item === 'object' && item !== null) { - handleReturnValue(item, warning); + handleReturnValue(rootDraft, item, warning); } }); + if (__DEV__ && warning && !isContainDraft) { + console.warn( + `The return value does not contain any draft, please use 'rawReturn()' to wrap the return value to improve performance.` + ); + } } function getCurrent(target: any) { diff --git a/src/index.ts b/src/index.ts index 64f9e86..620f9be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,7 @@ export { apply } from './apply'; export { original } from './original'; export { current } from './current'; export { unsafe } from './unsafe'; -export { safeReturn } from './safeReturn'; +export { rawReturn } from './rawReturn'; export { isDraft } from './utils/draft'; export { isDraftable } from './utils/draft'; diff --git a/src/rawReturn.ts b/src/rawReturn.ts new file mode 100644 index 0000000..2bca9bc --- /dev/null +++ b/src/rawReturn.ts @@ -0,0 +1,40 @@ +import { RAW_RETURN_SYMBOL } from './constant'; + +/** + * Use rawReturn() to wrap the return value to skip the draft check and thus improve performance. + * + * ## Example + * + * ```ts + * import { create, rawReturn } from '../index'; + * + * const baseState = { foo: { bar: 'str' }, arr: [] }; + * const state = create( + * baseState, + * (draft) => { + * return rawReturn(baseState); + * }, + * ); + * expect(state).toBe(baseState); + * ``` + */ +export function rawReturn(value: T): T { + if (arguments.length === 0) { + throw new Error('rawReturn() must be called with a value.'); + } + if (arguments.length > 1) { + throw new Error('rawReturn() must be called with one argument.'); + } + if ( + __DEV__ && + value !== undefined && + (typeof value !== 'object' || value === null) + ) { + console.warn( + 'rawReturn() must be called with an object or `undefined`, other types do not need to be returned via rawReturn().' + ); + } + return { + [RAW_RETURN_SYMBOL]: [value], + } as never; +} diff --git a/src/safeReturn.ts b/src/safeReturn.ts deleted file mode 100644 index c66774b..0000000 --- a/src/safeReturn.ts +++ /dev/null @@ -1,24 +0,0 @@ -export const safeReturnValue: unknown[] = []; - -/** - * It is used as a safe return value to ensure that this value replaces the finalized value. - */ -export function safeReturn(value: T): T { - if (arguments.length === 0) { - throw new Error('safeReturn() must be called with a value.'); - } - if (arguments.length > 1) { - throw new Error('safeReturn() must be called with one argument.'); - } - if ( - __DEV__ && - value !== undefined && - (typeof value !== 'object' || value === null) - ) { - console.warn( - 'safeReturn() must be called with an object or undefined, other types do not need to be returned via safeReturn().' - ); - } - safeReturnValue[0] = value; - return value; -} diff --git a/test/immer/base.test.ts b/test/immer/base.test.ts index 3e25201..6075d3c 100644 --- a/test/immer/base.test.ts +++ b/test/immer/base.test.ts @@ -1,6 +1,6 @@ // @ts-nocheck 'use strict'; -import { create, original, isDraft, safeReturn } from '../../src'; +import { create, original, isDraft, rawReturn } from '../../src'; import deepFreeze from 'deep-freeze'; import * as lodash from 'lodash'; import { getType } from '../../src/utils'; @@ -1531,28 +1531,26 @@ function runBaseTest( const base = {}; const result = create(base, (s1) => // ! it's different from mutative - safeReturn( - create( - { s1 }, - (s2) => { - expect(original(s2.s1)).toBe(s1); - s2.n = 1; - s2.s1 = create( - { s2 }, - (s3) => { - expect(original(s3.s2)).toBe(s2); - expect(original(s3.s2.s1)).toBe(s2.s1); - return s3.s2.s1; - }, - // ! it's different from mutative - { enableAutoFreeze: false } - ); - }, - { + create( + { s1 }, + (s2) => { + expect(original(s2.s1)).toBe(s1); + s2.n = 1; + s2.s1 = create( + { s2 }, + (s3) => { + expect(original(s3.s2)).toBe(s2); + expect(original(s3.s2.s1)).toBe(s2.s1); + return s3.s2.s1; + }, // ! it's different from mutative - enableAutoFreeze: false, - } - ) + { enableAutoFreeze: false } + ); + }, + { + // ! it's different from mutative + enableAutoFreeze: false, + } ) ); expect(result.n).toBe(1); @@ -1805,8 +1803,7 @@ function runBaseTest( it('can return an object with two references to an unmodified draft', () => { const base = { a: {} }; const next = produce(base, (d) => { - // ! it's different from mutative - return safeReturn([d.a, d.a]); + return [d.a, d.a]; }); expect(next[0]).toBe(base.a); expect(next[0]).toBe(next[1]); @@ -1928,7 +1925,7 @@ function runBaseTest( produce(state, (draft) => { switch (action.type) { case 'SET_STARTING_DOTS': - return safeReturn(draft.availableStartingDots.map((a) => a)); + return draft.availableStartingDots.map((a) => a); default: break; } @@ -1951,9 +1948,9 @@ function runBaseTest( produce(state, (draft) => { switch (action.type) { case 'SET_STARTING_DOTS': - return safeReturn({ + return { dots: draft.availableStartingDots.map((a) => a), - }); + }; default: break; } @@ -2053,15 +2050,15 @@ function runBaseTest( expect(create(base, () => null)).toBe(null); expect(create(base, () => undefined)).toBe(3); expect(create(base, () => {})).toBe(3); - expect(create(base, () => safeReturn(undefined))).toBe(undefined); + expect(create(base, () => rawReturn(undefined))).toBe(undefined); expect(create({}, () => undefined)).toEqual({}); - expect(create({}, () => safeReturn(undefined))).toBe(undefined); - expect(create(3, () => safeReturn(undefined))).toBe(undefined); + expect(create({}, () => rawReturn(undefined))).toBe(undefined); + expect(create(3, () => rawReturn(undefined))).toBe(undefined); expect(create(() => undefined)({})).toEqual({}); - expect(create(() => safeReturn(undefined))({})).toBe(undefined); - expect(create(() => safeReturn(undefined))(3)).toBe(undefined); + expect(create(() => rawReturn(undefined))({})).toBe(undefined); + expect(create(() => rawReturn(undefined))(3)).toBe(undefined); }); describe('base state type', () => { diff --git a/test/immer/patch.test.ts b/test/immer/patch.test.ts index ee56261..6a4ea4a 100644 --- a/test/immer/patch.test.ts +++ b/test/immer/patch.test.ts @@ -1,5 +1,5 @@ 'use strict'; -import { apply, create, isDraft, safeReturn } from '../../src'; +import { apply, create, isDraft, rawReturn } from '../../src'; jest.setTimeout(1000); @@ -1403,7 +1403,7 @@ test('#648 assigning object to itself should not change patches', () => { test('#791 patch for returning `undefined` is stored as undefined', () => { const [newState, patches] = create( { abc: 123 }, - (draft) => safeReturn(undefined), + (draft) => rawReturn(undefined), { enablePatches: true, } @@ -1447,7 +1447,7 @@ test('#888 patch to a primitive produces the primitive', () => { { const [res, patches] = create( { abc: 123 }, - (draft) => safeReturn(undefined), + (draft) => rawReturn(undefined), { enablePatches: true, } @@ -1456,35 +1456,35 @@ test('#888 patch to a primitive produces the primitive', () => { expect(patches).toEqual([{ op: 'replace', path: [], value: undefined }]); } { - const [res, patches] = create(null, (draft) => safeReturn(undefined), { + const [res, patches] = create(null, (draft) => rawReturn(undefined), { enablePatches: true, }); expect(res).toEqual(undefined); expect(patches).toEqual([{ op: 'replace', path: [], value: undefined }]); } { - const [res, patches] = create(0, (draft) => safeReturn(undefined), { + const [res, patches] = create(0, (draft) => rawReturn(undefined), { enablePatches: true, }); expect(res).toEqual(undefined); expect(patches).toEqual([{ op: 'replace', path: [], value: undefined }]); } { - const [res, patches] = create('foobar', (draft) => safeReturn(undefined), { + const [res, patches] = create('foobar', (draft) => rawReturn(undefined), { enablePatches: true, }); expect(res).toEqual(undefined); expect(patches).toEqual([{ op: 'replace', path: [], value: undefined }]); } { - const [res, patches] = create([], (draft) => safeReturn(undefined), { + const [res, patches] = create([], (draft) => rawReturn(undefined), { enablePatches: true, }); expect(res).toEqual(undefined); expect(patches).toEqual([{ op: 'replace', path: [], value: undefined }]); } { - const [res, patches] = create(false, (draft) => safeReturn(undefined), { + const [res, patches] = create(false, (draft) => rawReturn(undefined), { enablePatches: true, }); expect(res).toEqual(undefined); diff --git a/test/immer/produce.test.ts b/test/immer/produce.test.ts index eb84e7b..4ddd81a 100644 --- a/test/immer/produce.test.ts +++ b/test/immer/produce.test.ts @@ -5,8 +5,7 @@ import { Immutable, apply, castDraft, - castImmutable, - safeReturn, + rawReturn, } from '../../src'; interface State { @@ -269,12 +268,12 @@ it('can produce an undefined value', () => { const base = { a: 0 } as State; // Return only nothing. - let result = create(base, (_) => safeReturn(undefined)); + let result = create(base, (_) => rawReturn(undefined)); assert(result, _ as State); // Return maybe nothing. let result2 = create(base, (draft) => { - if (draft?.a ?? 0 > 0) return safeReturn(undefined); + if (draft?.a ?? 0 > 0) return rawReturn(undefined); }); assert(result2, _ as State); }); @@ -742,21 +741,21 @@ it('infers async curried', async () => { { // nothing allowed const res = create(base as State | undefined, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); assert(res, _ as State | undefined); } { // as any const res = create(base as State, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); assert(res, _ as State); } { // nothing not allowed create(base as State, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); } { diff --git a/test/index.test.ts b/test/index.test.ts index d78d88a..43c2c83 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2697,5 +2697,5 @@ test('can return an object that references itself', () => { expect(() => { // @ts-expect-error create(res, (draft) => res.self, { enableAutoFreeze: true }); - }).toThrowErrorMatchingInlineSnapshot(`"Forbids circular reference"`); + }).toThrowErrorMatchingInlineSnapshot(`"Maximum call stack size exceeded"`); }); diff --git a/test/performance/benchmark.ts b/test/performance/benchmark.ts index 933e4ba..efd7cd3 100644 --- a/test/performance/benchmark.ts +++ b/test/performance/benchmark.ts @@ -1,3 +1,4 @@ +/* eslint-disable import/no-relative-packages */ /* eslint-disable prefer-template */ // @ts-nocheck import fs from 'fs'; @@ -5,13 +6,13 @@ import https from 'https'; import { Suite } from 'benchmark'; import { parse } from 'json2csv'; import deepFreeze from 'deep-freeze'; -import produce, { +import QuickChart from 'quickchart-js'; +import { + produce, enablePatches, produceWithPatches, setAutoFreeze, - setUseProxies, -} from 'immer'; -import QuickChart from 'quickchart-js'; +} from '../../../temp/immer/dist'; import { create } from '../..'; const labels = []; @@ -105,7 +106,7 @@ suite { onStart: () => { setAutoFreeze(false); - setUseProxies(true); + // setUseProxies(true); i = Math.random(); baseState = getData(); }, @@ -144,7 +145,7 @@ suite { onStart: () => { setAutoFreeze(true); - setUseProxies(true); + // setUseProxies(true); i = Math.random(); baseState = getData(); }, @@ -183,7 +184,7 @@ suite { onStart: () => { setAutoFreeze(false); - setUseProxies(true); + // setUseProxies(true); enablePatches(); i = Math.random(); baseState = getData(); @@ -223,7 +224,7 @@ suite { onStart: () => { setAutoFreeze(true); - setUseProxies(true); + // setUseProxies(true); enablePatches(); i = Math.random(); baseState = getData(); diff --git a/test/performance/benchmarks/justReturn.ts b/test/performance/benchmarks/rawReturn.ts similarity index 95% rename from test/performance/benchmarks/justReturn.ts rename to test/performance/benchmarks/rawReturn.ts index 18e3cc0..fc730a7 100644 --- a/test/performance/benchmarks/justReturn.ts +++ b/test/performance/benchmarks/rawReturn.ts @@ -12,7 +12,7 @@ import produce, { setAutoFreeze, setUseProxies, } from 'immer'; -import { create, safeReturn } from '../../..'; +import { create, rawReturn } from '../../..'; const getData = () => { const baseState: { arr: any[]; map: Record } = { @@ -49,9 +49,9 @@ suite () => { const state = create(baseState, (draft) => { draft.arr; - return { + return rawReturn({ ...baseState, - }; + }); }); }, { @@ -87,9 +87,9 @@ suite baseState, (draft) => { draft.arr; - return { + return rawReturn({ ...baseState, - }; + }); }, { enableAutoFreeze: true, @@ -130,9 +130,9 @@ suite baseState, (draft) => { draft.arr; - return { + return rawReturn({ ...baseState, - }; + }); }, { enableAutoFreeze: false, @@ -174,9 +174,9 @@ suite baseState, (draft) => { draft.arr; - return { + return rawReturn({ ...baseState, - }; + }); }, { enableAutoFreeze: true, diff --git a/test/performance/benchmarks/returnWithDraft.ts b/test/performance/benchmarks/returnWithDraft.ts index c465151..0453ef0 100644 --- a/test/performance/benchmarks/returnWithDraft.ts +++ b/test/performance/benchmarks/returnWithDraft.ts @@ -11,7 +11,7 @@ import produce, { setAutoFreeze, setUseProxies, } from 'immer'; -import { create, safeReturn } from '../../..'; +import { create } from '../../..'; const getData = () => { const baseState: { arr: any[]; map: Record } = { @@ -46,13 +46,11 @@ suite .add( 'Mutative - No Freeze(by default)', () => { - const state = create(baseState, (draft) => - safeReturn({ - ...baseState, - arr: [...draft.arr, i], - map: { ...draft.map, [i]: { i } }, - }) - ); + const state = create(baseState, (draft) => ({ + ...baseState, + arr: [...draft.arr, i], + map: { ...draft.map, [i]: { i } }, + })); }, { onStart: () => { @@ -84,12 +82,11 @@ suite () => { const state = create( baseState, - (draft) => - safeReturn({ - ...baseState, - arr: [...draft.arr, i], - map: { ...draft.map, [i]: { i } }, - }), + (draft) => ({ + ...baseState, + arr: [...draft.arr, i], + map: { ...draft.map, [i]: { i } }, + }), { enableAutoFreeze: true, enablePatches: false, @@ -126,12 +123,11 @@ suite () => { const state = create( baseState, - (draft) => - safeReturn({ - ...baseState, - arr: [...draft.arr, i], - map: { ...draft.map, [i]: { i } }, - }), + (draft) => ({ + ...baseState, + arr: [...draft.arr, i], + map: { ...draft.map, [i]: { i } }, + }), { enableAutoFreeze: false, enablePatches: true, @@ -169,12 +165,11 @@ suite () => { const state = create( baseState, - (draft) => - safeReturn({ - ...baseState, - arr: [...draft.arr, i], - map: { ...draft.map, [i]: { i } }, - }), + (draft) => ({ + ...baseState, + arr: [...draft.arr, i], + map: { ...draft.map, [i]: { i } }, + }), { enableAutoFreeze: true, enablePatches: true, diff --git a/test/safeReturn.test.ts b/test/rawReturn.test.ts similarity index 58% rename from test/safeReturn.test.ts rename to test/rawReturn.test.ts index e6ef878..cd78052 100644 --- a/test/safeReturn.test.ts +++ b/test/rawReturn.test.ts @@ -6,7 +6,7 @@ /* eslint-disable no-self-assign */ /* eslint-disable no-param-reassign */ /* eslint-disable @typescript-eslint/ban-ts-comment */ -import { create, safeReturn } from '../src'; +import { create, isDraft, rawReturn } from '../src'; test('base', () => { const base = 3; @@ -15,47 +15,47 @@ test('base', () => { expect(create(base, () => null)).toBe(null); expect(create(base, () => undefined)).toBe(3); expect(create(base, () => {})).toBe(3); - expect(create(base, () => safeReturn(undefined))).toBe(undefined); + expect(create(base, () => rawReturn(undefined))).toBe(undefined); expect(create({}, () => undefined)).toEqual({}); - expect(create({}, () => safeReturn(undefined))).toBe(undefined); - expect(create(3, () => safeReturn(undefined))).toBe(undefined); + expect(create({}, () => rawReturn(undefined))).toBe(undefined); + expect(create(3, () => rawReturn(undefined))).toBe(undefined); expect(create(() => undefined)({})).toEqual({}); - expect(create(() => safeReturn(undefined))({})).toBe(undefined); - expect(create(() => safeReturn(undefined))(3)).toBe(undefined); + expect(create(() => rawReturn(undefined))({})).toBe(undefined); + expect(create(() => rawReturn(undefined))(3)).toBe(undefined); }); test('base enableAutoFreeze: true', () => { create( { a: { b: 1 } }, (draft) => { - return safeReturn({ + return { a: draft.a, - }); + }; }, { enableAutoFreeze: true } ); }); -test('base enableAutoFreeze: true - without safeReturn()', () => { +test('base enableAutoFreeze: true - with rawReturn()', () => { expect(() => { create( { a: { b: 1 } }, (draft) => { - return { + return rawReturn({ a: draft.a, - }; + }); }, { enableAutoFreeze: true } ); }).toThrowError(); }); -describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( - 'check Primitive type $useSafeReturn', - ({ useSafeReturn }) => { - test(`useSafeReturn ${useSafeReturn}: check Primitive type with returning`, () => { +describe.each([{ useRawReturn: true }, { useRawReturn: false }])( + 'check Primitive type $useRawReturn', + ({ useRawReturn }) => { + test(`useRawReturn ${useRawReturn}: check Primitive type with returning`, () => { [ -1, 1, @@ -72,13 +72,13 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( ].forEach((value: any) => { expect( create(value, (draft) => { - return useSafeReturn ? safeReturn(undefined) : ''; + return useRawReturn ? rawReturn(undefined) : ''; }) - ).toBe(useSafeReturn ? undefined : ''); + ).toBe(useRawReturn ? undefined : ''); }); }); - test(`useSafeReturn ${useSafeReturn}: check Primitive type with returning and patches`, () => { + test(`useRawReturn ${useRawReturn}: check Primitive type with returning and patches`, () => { [ -1, 1, @@ -98,21 +98,21 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( create( value, (draft) => { - return useSafeReturn ? safeReturn(undefined) : ''; + return useRawReturn ? rawReturn(undefined) : ''; }, { enablePatches: true, } ) ).toEqual([ - useSafeReturn ? undefined : '', - [{ op: 'replace', path: [], value: useSafeReturn ? undefined : '' }], + useRawReturn ? undefined : '', + [{ op: 'replace', path: [], value: useRawReturn ? undefined : '' }], [{ op: 'replace', path: [], value: value }], ]); }); }); - test(`useSafeReturn ${useSafeReturn}: check Primitive type with returning, patches and freeze`, () => { + test(`useRawReturn ${useRawReturn}: check Primitive type with returning, patches and freeze`, () => { [ -1, 1, @@ -132,7 +132,7 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( create( value, (draft) => { - return useSafeReturn ? safeReturn(undefined) : ''; + return useRawReturn ? rawReturn(undefined) : ''; }, { enableAutoFreeze: true, @@ -140,14 +140,14 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( } ) ).toEqual([ - useSafeReturn ? undefined : '', - [{ op: 'replace', path: [], value: useSafeReturn ? undefined : '' }], + useRawReturn ? undefined : '', + [{ op: 'replace', path: [], value: useRawReturn ? undefined : '' }], [{ op: 'replace', path: [], value: value }], ]); }); }); - test(`useSafeReturn ${useSafeReturn}: check Primitive type with returning, patches, freeze and async`, async () => { + test(`useRawReturn ${useRawReturn}: check Primitive type with returning, patches, freeze and async`, async () => { for (const value of [ -1, 1, @@ -167,7 +167,7 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( await create( value, async (draft) => { - return useSafeReturn ? safeReturn(undefined) : ''; + return useRawReturn ? rawReturn(undefined) : ''; }, { enableAutoFreeze: true, @@ -175,8 +175,8 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( } ) ).toEqual([ - useSafeReturn ? undefined : '', - [{ op: 'replace', path: [], value: useSafeReturn ? undefined : '' }], + useRawReturn ? undefined : '', + [{ op: 'replace', path: [], value: useRawReturn ? undefined : '' }], [{ op: 'replace', path: [], value: value }], ]); } @@ -187,16 +187,16 @@ describe.each([{ useSafeReturn: true }, { useSafeReturn: false }])( test('error args', () => { expect(() => // @ts-expect-error - create(3, () => safeReturn(undefined, undefined)) + create(3, () => rawReturn(undefined, undefined)) ).toThrowErrorMatchingInlineSnapshot( - `"safeReturn() must be called with one argument."` + `"rawReturn() must be called with one argument."` ); expect(() => // @ts-expect-error - create({}, () => safeReturn()) + create({}, () => rawReturn()) ).toThrowErrorMatchingInlineSnapshot( - `"safeReturn() must be called with a value."` + `"rawReturn() must be called with a value."` ); const logSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); @@ -215,9 +215,9 @@ test('error args', () => { true, Symbol('foo'), ].forEach((value: any) => { - create({}, () => safeReturn(value)); + create({}, () => rawReturn(value)); expect(logSpy).toHaveBeenCalledWith( - 'safeReturn() must be called with an object or undefined, other types do not need to be returned via safeReturn().' + 'rawReturn() must be called with an object or `undefined`, other types do not need to be returned via rawReturn().' ); logSpy.mockClear(); }); @@ -225,49 +225,53 @@ test('error args', () => { logSpy.mockReset(); }); -test('check warning in strict mode', () => { +test('check warning rawReturn() in strict mode', () => { class Foo { a?: any; } const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); [ (draft: any) => { - return { + return rawReturn({ a: draft.a, - }; + }); + }, + (draft: any) => { + return rawReturn([draft.a]); }, (draft: any) => { - return [draft.a]; + return rawReturn( + new Map([ + [ + 1, + { + a: draft.a, + }, + ], + ]) + ); }, (draft: any) => { - return new Map([ - [ + return rawReturn( + new Set([ 1, { a: draft.a, }, - ], - ]); - }, - (draft: any) => { - return new Set([ - 1, - { - a: draft.a, - }, - ]); + ]) + ); }, (draft: any) => { const foo = new Foo(); foo.a = draft.a; - return foo; + return rawReturn(foo); }, ].forEach((callback: any) => { create({ a: { b: 1 } }, callback, { strict: true, }); expect(warnSpy).toHaveBeenCalledWith( - `The return value contains drafts, please use safeReturn() to wrap the return value.` + `The return value contains drafts, please don't use 'rawReturn()' to wrap the return value.` ); warnSpy.mockClear(); }); @@ -283,20 +287,36 @@ test('return parent draft', () => { }) ).toEqual({ a: 2 }); }); +test('mix more type draft without rawReturn()', () => { + [ + (draft: any) => ({ + a: { + b: draft.a, + }, + }), + (draft: any) => [{ c: draft.a }], + (draft: any) => new Map([[1, draft.a]]), + (draft: any) => new Set([1, draft.a]), + ].forEach((callback: any) => { + expect(() => create({ a: { b: 1 } }, callback)).not.toThrowError(); + }); +}); -test('mix more type draft', () => { +test('mix more type draft with rawReturn()', () => { [ (draft: any) => - safeReturn({ + rawReturn({ a: { b: draft.a, }, }), - (draft: any) => safeReturn([{ c: draft.a }]), - (draft: any) => safeReturn(new Map([[1, draft.a]])), - (draft: any) => safeReturn(new Set([1, draft.a])), + (draft: any) => rawReturn([{ c: draft.a }]), + (draft: any) => rawReturn(new Map([[1, draft.a]])), + (draft: any) => rawReturn(new Set([1, draft.a])), ].forEach((callback: any) => { - expect(() => create({ a: { b: 1 } }, callback)).not.toThrowError(); + expect(() => + expect(create({ a: { b: 1 } }, callback)).toEqual({}) + ).toThrowError(); }); }); @@ -317,11 +337,9 @@ test(`safe returning with non-enumerable or symbolic properties`, () => { const state2: any = create( state, (draft) => { - return safeReturn( - Object.assign(component, { - [key]: draft, - }) - ) as any; + return Object.assign(component, { + [key]: draft, + }) as any; }, { enableAutoFreeze: true, @@ -350,7 +368,7 @@ test('works with interweaved Immer instances with disable Freeze', () => { enableAutoFreeze: false, } ); - return safeReturn(f); + return f; }); // @ts-expect-error expect(result.s1).toBe(base); @@ -359,9 +377,9 @@ test('works with interweaved Immer instances with disable Freeze', () => { test('deep draft', () => { const state = create({ a: { b: { c: 1 } } }, (draft) => { draft.a.b.c; - return safeReturn({ + return { a: draft.a, - }); + }; }); expect(state).toEqual({ a: { b: { c: 1 } } }); }); @@ -369,7 +387,46 @@ test('deep draft', () => { test('case', () => { const baseState = { foo: 'bar' }; const state = create(baseState as { foo: string } | undefined, (draft) => { - return safeReturn(undefined); + return rawReturn(undefined); }); expect(state).toBe(undefined); }); + +test('does not finalize upvalue drafts', () => { + create({ a: {}, b: {} }, (parent) => { + expect(create({}, () => parent)).toBe(parent); + // @ts-ignore + parent.x; // Ensure proxy not revoked. + + // @ts-ignore + expect(create({}, () => [parent])[0]).toBe(parent); + // @ts-ignore + parent.x; // Ensure proxy not revoked. + + expect(create({}, () => parent.a)).toBe(parent.a); + // @ts-ignore + + parent.a.x; // Ensure proxy not revoked. + // @ts-ignore + // Modified parent test + parent.c = 1; + // @ts-ignore + expect(create({}, () => [parent.b])[0]).toBe(parent.b); + // @ts-ignore + parent.b.x; // Ensure proxy not revoked. + }); +}); + +test('mixed draft', () => { + const baseState = { a: 1, b: { c: 1 } }; + const state = create(baseState, (draft) => { + if (draft.b.c === 1) { + return { + ...draft, + a: 2, + }; + } + }); + expect(state).toEqual({ a: 2, b: { c: 1 } }); + expect(isDraft(state.b)).toBeFalsy(); +});