Skip to content

Commit

Permalink
fix(draft): fix finalization draft issue about assigning a non-draft (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
unadlib authored Nov 8, 2023
1 parent af1f573 commit f6ea7aa
Show file tree
Hide file tree
Showing 9 changed files with 647 additions and 8 deletions.
Binary file modified benchmark-array.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified benchmark-object.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified benchmark.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 4 additions & 1 deletion src/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ export function apply<T extends object, F extends boolean = false>(
let base: any = draft;
for (let index = 0; index < path.length - 1; index += 1) {
const parentType = getType(base);
const key = String(path[index]);
let key = path[index];
if (typeof key !== 'string' && typeof key !== 'number') {
key = String(key);
}
if (
((parentType === DraftType.Object ||
parentType === DraftType.Array) &&
Expand Down
18 changes: 17 additions & 1 deletion src/draft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export function createDraft<T extends object>(createDraftOptions: {
target.finalities.draft.push((patches, inversePatches) => {
const oldProxyDraft = getProxyDraft(proxy)!;
// if target is a Set draft, `setMap` is the real Set copies proxy mapping.
const copy = target.type === DraftType.Set ? target.setMap : target.copy;
let copy = target.type === DraftType.Set ? target.setMap : target.copy;
const draft = get(copy, key!);
const proxyDraft = getProxyDraft(draft);
if (proxyDraft) {
Expand All @@ -260,6 +260,22 @@ export function createDraft<T extends object>(createDraftOptions: {
target.options.updatedValues ?? new WeakMap();
target.options.updatedValues.set(updatedValue, proxyDraft.original);
}
if (
proxyDraft.parent!.key !== undefined &&
proxyDraft.parent!.parent!.proxy
) {
const parent =
proxyDraft.parent!.parent!.proxy[proxyDraft.parent!.key];
const parentProxyDraft = getProxyDraft(parent);
if (parentProxyDraft === undefined) {
// !case: handle assigning a non-draft with the same key
copy = parent;
const current = get(copy, key!);
if (!getProxyDraft(current)) {
updatedValue = current;
}
}
}
// final update value
set(copy, key!, updatedValue);
}
Expand Down
3 changes: 2 additions & 1 deletion src/utils/draft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ export function get(target: any, key: PropertyKey) {
}

export function set(target: any, key: PropertyKey, value: any) {
if (getType(target) === DraftType.Map) {
const type = getType(target);
if (type === DraftType.Map) {
target.set(key, value);
} else {
target[key] = value;
Expand Down
8 changes: 5 additions & 3 deletions src/utils/finalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ export function handleValue(target: any, handledSet: WeakSet<any>) {
if (isDraft(value)) {
const proxyDraft = getProxyDraft(value)!;
ensureShallowCopy(proxyDraft);
const updatedValue = proxyDraft.assignedMap?.size
? proxyDraft.copy
: proxyDraft.original;
// A draft where a child node has been changed, or assigned a value
const updatedValue =
proxyDraft.assignedMap?.size || proxyDraft.operated
? proxyDraft.copy
: proxyDraft.original;
// final update value
set(isSet ? setMap! : target, key, updatedValue);
} else {
Expand Down
74 changes: 72 additions & 2 deletions test/immer-non-support.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@
/* eslint-disable prefer-destructuring */
/* eslint-disable no-param-reassign */
/* eslint-disable no-lone-blocks */
import { produce, enableMapSet, setAutoFreeze, Immutable } from 'immer';
import { create } from '../src';
import {
produce,
enableMapSet,
setAutoFreeze,
Immutable,
produceWithPatches,
enablePatches,
applyPatches,
} from 'immer';
import { create, apply } from '../src';

enableMapSet();

Expand Down Expand Up @@ -303,3 +311,65 @@ test('circular reference', () => {
);
}
});

test('#18 - set: assigning a non-draft with the same key', () => {
const baseState = {
array: [
{
one: {
two: {
three: 3,
},
},
},
],
};

const created = create(
baseState,
(draft) => {
draft.array[0].one.two.three = 2;
const two = draft.array[0].one.two;
const one = new Set();
// @ts-ignore
draft.array = [{ one }];
// @ts-ignore
one.add(two);
// @ts-ignore
expect(Array.from(draft.array[0].one)[0].three).toBe(2);
},
{
enablePatches: true,
}
);
// @ts-ignore
expect(Array.from(created[0].array[0].one)[0].three).toBe(2);
expect(apply(baseState, created[1])).toEqual(created[0]);
// expect(apply(created[0], created[2])).toEqual(baseState);

enablePatches();
// @ts-ignore
const produced = produceWithPatches(baseState, (draft: any) => {
draft.array[0].one.two.three = 2;
const two = draft.array[0].one.two;
const one = new Set();
// @ts-ignore
draft.array = [{ one }];
// @ts-ignore
one.add(two);
// @ts-ignore
expect(Array.from(draft.array[0].one)[0].three).toBe(2);
});

// @ts-ignore
expect(() => {
// @ts-ignore
// eslint-disable-next-line no-unused-expressions
Array.from(produced[0].array[0].one)[0].three;
}).toThrowError();

// @ts-ignore
// expect(applyPatches(baseState, produced[1])).toEqual(produced[0]);
// @ts-ignore
// expect(applyPatches(produced[0], produced[2])).toEqual(baseState);
});
Loading

0 comments on commit f6ea7aa

Please sign in to comment.