Skip to content

Commit

Permalink
Add Optional Support For Multiple References to an Object
Browse files Browse the repository at this point in the history
Some state trees may need to reference an object more than once (such as the tree for my [fomod](https://www.npmjs.com/package/fomod) library. By using a combination of a WeakMap to store existing drafts and an off-by-default configuration option, this should be a painless solution. I've tested it within the scope of my project but there may always be issues I haven't foreseen.
  • Loading branch information
BellCubeDev committed Feb 4, 2024
1 parent f6736a4 commit eebb9e2
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 37 deletions.
50 changes: 36 additions & 14 deletions src/core/finalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ import {
getPlugin,
die,
revokeScope,
isFrozen
isFrozen,
type Objectish,
type Drafted,
prepareCopy
} from "../internal"

export function processResult(result: any, scope: ImmerScope) {
export function processResult(result: any, scope: ImmerScope, existingStateMap?: WeakMap<Objectish, ImmerState>, existingFinalizationMap?: WeakMap<Objectish, Drafted>) {
scope.unfinalizedDrafts_ = scope.drafts_.length
const baseDraft = scope.drafts_![0]
const isReplaced = result !== undefined && result !== baseDraft
Expand All @@ -29,8 +32,8 @@ export function processResult(result: any, scope: ImmerScope) {
}
if (isDraftable(result)) {
// Finalize the result in case it contains (or is) a subset of the draft.
result = finalize(scope, result)
if (!scope.parent_) maybeFreeze(scope, result)
result = finalize(scope, result, undefined, existingStateMap, existingFinalizationMap)
if (!scope.parent_) maybeFreeze(scope, result, false)
}
if (scope.patches_) {
getPlugin("Patches").generateReplacementPatches_(
Expand All @@ -42,7 +45,7 @@ export function processResult(result: any, scope: ImmerScope) {
}
} else {
// Finalize the base draft.
result = finalize(scope, baseDraft, [])
result = finalize(scope, baseDraft, [], existingStateMap, existingFinalizationMap)
}
revokeScope(scope)
if (scope.patches_) {
Expand All @@ -51,17 +54,19 @@ export function processResult(result: any, scope: ImmerScope) {
return result !== NOTHING ? result : undefined
}

function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
function finalize(rootScope: ImmerScope, value: any, path?: PatchPath, existingStateMap?: WeakMap<Objectish, ImmerState>, existingFinalizationMap?: WeakMap<Objectish, Drafted>): any {
// Don't recurse in tho recursive data structures
if (isFrozen(value)) return value

const state: ImmerState = value[DRAFT_STATE]
let state: ImmerState = value[DRAFT_STATE]

// A plain object, might need freezing, might contain drafts
if (!state) {
existingFinalizationMap?.set(value, value)
each(
value,
(key, childValue) =>
finalizeProperty(rootScope, state, value, key, childValue, path),
finalizeProperty(rootScope, state, value, key, childValue, path, undefined, existingStateMap, existingFinalizationMap),
true // See #590, don't recurse into non-enumerable of non drafted objects
)
return value
Expand All @@ -70,11 +75,13 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
if (state.scope_ !== rootScope) return value
// Unmodified draft, return the (frozen) original
if (!state.modified_) {
maybeFreeze(rootScope, state.base_, true)
maybeFreeze(rootScope, state.copy_ ?? state.base_, true);
return state.base_
}
// Not finalized yet, let's do that now
if (!state.finalized_) {
existingFinalizationMap?.set(state.base_, state.copy_)

state.finalized_ = true
state.scope_.unfinalizedDrafts_--
const result = state.copy_
Expand All @@ -90,7 +97,7 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
isSet = true
}
each(resultEach, (key, childValue) =>
finalizeProperty(rootScope, state, result, key, childValue, path, isSet)
finalizeProperty(rootScope, state, result, key, childValue, path, isSet, existingStateMap, existingFinalizationMap)
)
// everything inside is frozen, we can freeze here
maybeFreeze(rootScope, result, false)
Expand All @@ -104,6 +111,7 @@ function finalize(rootScope: ImmerScope, value: any, path?: PatchPath) {
)
}
}

return state.copy_
}

Expand All @@ -114,10 +122,23 @@ function finalizeProperty(
prop: string | number,
childValue: any,
rootPath?: PatchPath,
targetIsSet?: boolean
targetIsSet?: boolean,
existingStateMap?: WeakMap<Objectish, ImmerState>,
existingFinalizationMap?: WeakMap<Objectish, Drafted>,
) {
if (process.env.NODE_ENV !== "production" && childValue === targetObject)
die(5)

if (!isDraft(childValue) && isDraftable(childValue)) {
const existingState = existingStateMap?.get(childValue)
if (existingState) {
childValue = existingState.draft_
} else {
const existingFinalization = existingFinalizationMap?.get(childValue)
if (existingFinalization) return set(targetObject, prop, existingFinalization)
}
}

if (isDraft(childValue)) {
const path =
rootPath &&
Expand All @@ -127,7 +148,7 @@ function finalizeProperty(
? rootPath!.concat(prop)
: undefined
// Drafts owned by `scope` are finalized here.
const res = finalize(rootScope, childValue, path)
const res = finalize(rootScope, childValue, path, existingStateMap, existingFinalizationMap)
set(targetObject, prop, res)
// Drafts from another scope must prevented to be frozen
// if we got a draft back from finalize, we're in a nested produce and shouldn't freeze
Expand All @@ -137,6 +158,7 @@ function finalizeProperty(
} else if (targetIsSet) {
targetObject.add(childValue)
}

// Search new objects for unfinalized drafts. Frozen objects should never contain drafts.
if (isDraftable(childValue) && !isFrozen(childValue)) {
if (!rootScope.immer_.autoFreeze_ && rootScope.unfinalizedDrafts_ < 1) {
Expand All @@ -147,10 +169,10 @@ function finalizeProperty(
// See add-data.js perf test
return
}
finalize(rootScope, childValue)
finalize(rootScope, childValue, undefined, existingStateMap, existingFinalizationMap)
// immer deep freezes plain objects, so if there is no parent state, we freeze as well
if (!parentState || !parentState.scope_.parent_)
maybeFreeze(rootScope, childValue)
maybeFreeze(rootScope, childValue, false);
}
}

Expand Down
34 changes: 26 additions & 8 deletions src/core/immerClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,19 @@ interface ProducersFns {
export class Immer implements ProducersFns {
autoFreeze_: boolean = true
useStrictShallowCopy_: boolean = false
allowMultiRefs_: boolean = false

constructor(config?: {autoFreeze?: boolean; useStrictShallowCopy?: boolean}) {
constructor(config?: {
autoFreeze?: boolean
useStrictShallowCopy?: boolean
allowMultiRefs: boolean
}) {
if (typeof config?.autoFreeze === "boolean")
this.setAutoFreeze(config!.autoFreeze)
if (typeof config?.useStrictShallowCopy === "boolean")
this.setUseStrictShallowCopy(config!.useStrictShallowCopy)
if (typeof config?.allowMultiRefs === "boolean")
this.setAllowMultiRefs(config!.allowMultiRefs)
}

/**
Expand Down Expand Up @@ -86,7 +93,8 @@ export class Immer implements ProducersFns {
// Only plain objects, arrays, and "immerable classes" are drafted.
if (isDraftable(base)) {
const scope = enterScope(this)
const proxy = createProxy(base, undefined)
const stateMap = this.allowMultiRefs_ ? new Map() : undefined
const proxy = createProxy(base, undefined, stateMap)
let hasError = true
try {
result = recipe(proxy)
Expand All @@ -97,7 +105,7 @@ export class Immer implements ProducersFns {
else leaveScope(scope)
}
usePatchesInScope(scope, patchListener)
return processResult(result, scope)
return processResult(result, scope, stateMap, this.allowMultiRefs_ ? new WeakMap() : undefined)
} else if (!base || typeof base !== "object") {
result = recipe(base)
if (result === undefined) result = base
Expand Down Expand Up @@ -132,7 +140,7 @@ export class Immer implements ProducersFns {
if (!isDraftable(base)) die(8)
if (isDraft(base)) base = current(base)
const scope = enterScope(this)
const proxy = createProxy(base, undefined)
const proxy = createProxy(base, undefined, this.allowMultiRefs_ ? new WeakMap() : undefined)
proxy[DRAFT_STATE].isManual_ = true
leaveScope(scope)
return proxy as any
Expand All @@ -144,9 +152,11 @@ export class Immer implements ProducersFns {
): D extends Draft<infer T> ? T : never {
const state: ImmerState = draft && (draft as any)[DRAFT_STATE]
if (!state || !state.isManual_) die(9)
const {scope_: scope} = state

// @ts-ignore -- TODO: Remove this (add typings to state and map!)
const {scope_: scope, existingstateMap_} = state
usePatchesInScope(scope, patchListener)
return processResult(undefined, scope)
return processResult(undefined, scope, existingstateMap_, this.allowMultiRefs_ ? new WeakMap() : undefined) as any
}

/**
Expand All @@ -167,6 +177,11 @@ export class Immer implements ProducersFns {
this.useStrictShallowCopy_ = value
}

/** Pass true to allow multiple references to the same object in the same state tree. */
setAllowMultiRefs(value: boolean) {
this.allowMultiRefs_ = value
}

applyPatches<T extends Objectish>(base: T, patches: Patch[]): T {
// If a patch replaces the entire state, take that replacement as base
// before applying patches
Expand Down Expand Up @@ -198,16 +213,19 @@ export class Immer implements ProducersFns {

export function createProxy<T extends Objectish>(
value: T,
parent?: ImmerState
parent?: ImmerState,
stateMap?: WeakMap<Objectish, ImmerState>
): Drafted<T, ImmerState> {
// precondition: createProxy should be guarded by isDraftable, so we know we can safely draft
const draft: Drafted = isMap(value)
? getPlugin("MapSet").proxyMap_(value, parent)
: isSet(value)
? getPlugin("MapSet").proxySet_(value, parent)
: createProxyProxy(value, parent)
: createProxyProxy(value, parent, stateMap)

const scope = parent ? parent.scope_ : getCurrentScope()

scope.drafts_.push(draft)

return draft
}
39 changes: 26 additions & 13 deletions src/core/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ export interface ProxyObjectState extends ProxyBaseState {
base_: any
copy_: any
draft_: Drafted<AnyObject, ProxyObjectState>
stateMap_?: WeakMap<Objectish, ImmerState> | undefined
}

export interface ProxyArrayState extends ProxyBaseState {
type_: ArchType.Array
base_: AnyArray
copy_: AnyArray | null
draft_: Drafted<AnyArray, ProxyArrayState>
stateMap_?: WeakMap<Objectish, ImmerState> | undefined
}

type ProxyState = ProxyObjectState | ProxyArrayState
Expand All @@ -51,8 +53,10 @@ type ProxyState = ProxyObjectState | ProxyArrayState
*/
export function createProxyProxy<T extends Objectish>(
base: T,
parent?: ImmerState
parent?: ImmerState,
stateMap?: WeakMap<Objectish, ImmerState>
): Drafted<T, ProxyState> {

const isArray = Array.isArray(base)
const state: ProxyState = {
type_: isArray ? ArchType.Array : (ArchType.Object as any),
Expand All @@ -74,7 +78,8 @@ export function createProxyProxy<T extends Objectish>(
copy_: null,
// Called by the `produce` function.
revoke_: null as any,
isManual_: false
isManual_: false,
stateMap_: stateMap
}

// the traps must target something, a bit like the 'real' base.
Expand Down Expand Up @@ -116,7 +121,11 @@ export const objectTraps: ProxyHandler<ProxyState> = {
// Assigned values are never drafted. This catches any drafts we created, too.
if (value === peek(state.base_, prop)) {
prepareCopy(state)
return (state.copy_![prop as any] = createProxy(value, state))
return (state.copy_![prop as any] = createProxy(
value,
state,
state.stateMap_
))
}
return value
},
Expand Down Expand Up @@ -278,15 +287,19 @@ export function markChanged(state: ImmerState) {
}
}

export function prepareCopy(state: {
base_: any
copy_: any
scope_: ImmerScope
}) {
if (!state.copy_) {
state.copy_ = shallowCopy(
state.base_,
state.scope_.immer_.useStrictShallowCopy_
)
export function prepareCopy(state: ImmerState) {
if (state.copy_) return

const existing = state.stateMap_?.get(state.base_)
if (existing) {
Object.assign(state, existing)
return
}

state.copy_ = shallowCopy(
state.base_,
state.scope_.immer_.useStrictShallowCopy_
)

state.stateMap_?.set(state.base_, state)
}
8 changes: 6 additions & 2 deletions src/plugins/mapset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export function enableMapSet() {
base_: target,
draft_: this as any,
isManual_: false,
revoked_: false
revoked_: false,
stateMap_: parent?.stateMap_ as any
}
}

Expand Down Expand Up @@ -167,6 +168,7 @@ export function enableMapSet() {
if (!state.copy_) {
state.assigned_ = new Map()
state.copy_ = new Map(state.base_)
state.stateMap_?.set(state.base_, state)
}
}

Expand All @@ -185,7 +187,8 @@ export function enableMapSet() {
draft_: this,
drafts_: new Map(),
revoked_: false,
isManual_: false
isManual_: false,
stateMap_: parent?.stateMap_ as any
}
}

Expand Down Expand Up @@ -284,6 +287,7 @@ export function enableMapSet() {
if (!state.copy_) {
// create drafts for all entries to preserve insertion order
state.copy_ = new Set()
state.stateMap_?.set(state.base_, state)
state.base_.forEach(value => {
if (isDraftable(value)) {
const draft = createProxy(value, state)
Expand Down
2 changes: 2 additions & 0 deletions src/utils/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export interface MapState extends ImmerBaseState {
base_: AnyMap
revoked_: boolean
draft_: Drafted<AnyMap, MapState>
stateMap_?: WeakMap<AnyMap, ImmerState> | undefined
}

export interface SetState extends ImmerBaseState {
Expand All @@ -69,6 +70,7 @@ export interface SetState extends ImmerBaseState {
drafts_: Map<any, Drafted> // maps the original value to the draft value in the new set
revoked_: boolean
draft_: Drafted<AnySet, SetState>
stateMap_?: WeakMap<AnySet, ImmerState> | undefined
}

/** Patches plugin */
Expand Down
2 changes: 2 additions & 0 deletions website/docs/pitfalls.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Never reassign the `draft` argument (example: `draft = myCoolNewState`). Instead

### Immer only supports unidirectional trees

<!-- TODO: Discuss what to do in the docs regarding the multiple references PR -->

Immer assumes your state to be a unidirectional tree. That is, no object should appear twice in the tree, there should be no circular references. There should be exactly one path from the root to any node of the tree.

### Never explicitly return `undefined` from a producer
Expand Down

0 comments on commit eebb9e2

Please sign in to comment.