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

refactor(returning): refactor handling return value via rawReturn() and remove safeReturn() #11

Merged
merged 5 commits into from
Mar 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 44 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -302,19 +302,56 @@ 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.
For return values that do not contain any drafts, you can use `rawReturn()` to wrap this return value to improve performance. It ensure that the return value is only returned 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();
```

If you use `rawReturn()`, we recommend that you enable `strict` mode in development.

```ts
const baseState = { a: 1, b: { c: 1 } };
const state = create(
baseState,
(draft) => {
if (draft.b.c === 1) {
return rawReturn({
...draft,
a: 2,
});
}
},
{
strict: true,
}
);
// it will warn `The return value contains drafts, please don't use 'rawReturn()' to wrap the return value.` in strict mode.
expect(state).toEqual({ a: 2, b: { c: 1 } });
expect(isDraft(state.b)).toBeFalsy();
```

[View more API docs](./docs/README.md).

Expand Down Expand Up @@ -440,10 +477,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);
});
```

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
"immer": "^9.0.19",
"immutable": "^4.0.0",
"jest": "^29.4.0",
"jsdoc-tests": "^0.1.1",
"jsdoc-tests": "^1.1.0",
"json2csv": "^5.0.7",
"lodash": "^4.17.21",
"prettier": "^2.5.1",
Expand Down
1 change: 1 addition & 0 deletions src/constant.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
20 changes: 12 additions & 8 deletions src/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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' &&
Expand Down Expand Up @@ -146,16 +145,21 @@ 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({
rootDraft: proxyDraft,
value,
useRawReturn: 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({ rootDraft: proxyDraft, value });
}
return finalize([value]);
}
Expand Down
40 changes: 31 additions & 9 deletions src/current.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DraftType } from './interface';
import { DraftType, ProxyDraft } from './interface';
import {
forEach,
get,
Expand All @@ -11,15 +11,23 @@ import {
shallowCopy,
} from './utils';

export function handleReturnValue<T extends object>(value: T, warning = false) {
export function handleReturnValue<T extends object>(options: {
rootDraft: ProxyDraft<any> | undefined;
value: T;
useRawReturn?: boolean;
isContainDraft?: boolean;
isRoot?: boolean;
}) {
const { rootDraft, value, useRawReturn = false, isRoot = true } = options;
forEach(value, (key, item, source) => {
const proxyDraft = getProxyDraft(item);
if (proxyDraft) {
if (__DEV__ && warning) {
console.warn(
`The return value contains drafts, please use safeReturn() to wrap the return value.`
);
}
// just handle the draft which is created by the same rootDraft
if (
proxyDraft &&
rootDraft &&
proxyDraft.finalities === rootDraft.finalities
) {
options.isContainDraft = true;
const currentValue = proxyDraft.original;
if (source instanceof Set) {
const arr = Array.from(source);
Expand All @@ -31,9 +39,23 @@ export function handleReturnValue<T extends object>(value: T, warning = false) {
set(source, key, currentValue);
}
} else if (typeof item === 'object' && item !== null) {
handleReturnValue(item, warning);
options.value = item;
options.isRoot = false;
handleReturnValue(options);
}
});
if (__DEV__ && isRoot) {
if (!options.isContainDraft)
console.warn(
`The return value does not contain any draft, please use 'rawReturn()' to wrap the return value to improve performance.`
);

if (useRawReturn) {
console.warn(
`The return value contains drafts, please don't use 'rawReturn()' to wrap the return value.`
);
}
}
}

function getCurrent(target: any) {
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
40 changes: 40 additions & 0 deletions src/rawReturn.ts
Original file line number Diff line number Diff line change
@@ -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<T extends object | undefined>(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(including plain object, arrays, Set, Map, etc.) or `undefined`, other types do not need to be returned via rawReturn().'
);
}
return {
[RAW_RETURN_SYMBOL]: [value],
} as never;
}
24 changes: 0 additions & 24 deletions src/safeReturn.ts

This file was deleted.

63 changes: 30 additions & 33 deletions test/immer/base.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -1522,7 +1522,7 @@ function runBaseTest(

// Modified parent test
parent.c = 1;
expect(produce({}, () => [parent.b])[0]).toBe(parent.b);
expect(produce(null, () => [parent.b])[0]).toBe(parent.b);
parent.b.x; // Ensure proxy not revoked.
});
});
Expand All @@ -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);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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', () => {
Expand Down
Loading