-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem: Effects may leak Immer drafts from event handlers
Context
Our event handlers run inside an Immer produce scope and may return an effect tuple like:
return [[EFFECT_IDS.LOCAL_STORAGE_SET, { key: 'userAnswers', value: current(draftDb.userAnswers) }]];The second element of an effect can be any object.
Currently, if that object (or any nested value) references an Immer draft (e.g. draftDb.*), the handler author must manually wrap values with current() to avoid returning revoked drafts.
This is error-prone and inconsistent.
Current behavior
- Returning payloads that reference drafts will either:
- throw when accessed outside the producer (revoked draft), or
- silently carry proxies with unexpected semantics.
- Developers must remember to call
current()(or deep clone) themselves.
Desired behavior
- The library guarantees that all effects returned from handlers are draft-free, plain data (POJOs, arrays, Maps/Sets with plain contents).
- Handler authors can return natural references to
draftDbwithout manualcurrent()calls.
Proposed solution
Add an internal finalization step that deep-unwraps any drafts in the effect payloads before they leave the produce scope.
Implementation outline
Implement finalizeEffectPayload(value: unknown): unknown that:
- Detects drafts with
isDraft(value). - Converts drafts to plain snapshots using
current(value)(or equivalent). - Recursively processes Arrays, Objects, Maps, and Sets.
- Guards against cycles via
WeakSet. - Preserves symbol keys.
Apply this finalizer to:
- every effect payload returned from a handler, and
- optionally to any library-generated payloads.
Optionally expose a config flag:
createStore({ autoFinalizeEffects: true }) // default: truefor backwards compatibility.
Example
Handler code (no manual current() required)
regEvent(EVENT_IDS.ANSWER_QUESTION, ({ draftDb }, questionIndex, answerIndex) => {
if (draftDb.selectedCategory === 'test') {
draftDb.testAnswers[questionIndex] = answerIndex;
} else {
draftDb.userAnswers[questionIndex] = answerIndex;
return [[EFFECT_IDS.LOCAL_STORAGE_SET, { key: 'userAnswers', value: draftDb.userAnswers }]];
}
});Library finalization (conceptual implementation)
import { isDraft, current, isDraftable } from "immer";
function finalizeEffectPayload(value: unknown, seen = new WeakSet<object>()): any {
if (isDraft(value as any)) return current(value as any);
if (value === null || typeof value !== "object") return value;
const obj = value as object;
if (seen.has(obj)) return value;
seen.add(obj);
if (value instanceof Map) {
const out = new Map();
for (const [k, v] of value) {
out.set(finalizeEffectPayload(k, seen), finalizeEffectPayload(v, seen));
}
return out;
}
if (value instanceof Set) {
const out = new Set();
for (const v of value) out.add(finalizeEffectPayload(v, seen));
return out;
}
if (Array.isArray(value)) {
return (value as unknown[]).map(v => finalizeEffectPayload(v, seen));
}
if (isDraftable(value as any)) {
const out: any = {};
for (const key of Object.keys(value as any)) {
out[key] = finalizeEffectPayload((value as any)[key], seen);
}
for (const sym of Object.getOwnPropertySymbols(obj)) {
out[sym as any] = finalizeEffectPayload((value as any)[sym as any], seen);
}
return out;
}
return value;
}
// When processing handler results:
effects = effects.map(([id, payload]) => [id, finalizeEffectPayload(payload)]);Alternatives considered
-
Document the requirement to always use
current()
→ Puts burden on users; easy to miss. -
Use
JSON.parse(JSON.stringify(...))
→ Loses types, drops non-JSON values (Dates, Maps/Sets, symbols), and is slow. -
Use
original()
→ Not correct for snapshots of modified drafts.
Risks and edge cases
- Performance: Deep traversal on large payloads — mitigate by short-circuiting when no drafts detected at the top level.
- Non-serializable values: Functions, class instances — pass through untouched; document that consumers must handle them.
Acceptance criteria
- Handlers can return effect payloads that reference drafts directly without using
current(). - No revoked-draft errors occur when consuming effects after the producer ends.
- Unit tests:
- Plain object with nested draft is finalized.
- Arrays/Maps/Sets containing drafts are finalized.
- Symbol keys are preserved.
- Cyclic graphs do not throw.
- Payloads without drafts are returned as-is (identity behavior where possible).
- Documentation updated with the new guarantee and the
autoFinalizeEffectsoption (default:true).
Additional API options
6) Return an effect builder function: (appDb: Db) => Effects
Idea: a handler may return a function that will be invoked after the producer finishes, with a plain (non-draft) appDb. The function returns final effects.
Usage:
regEventFx(EVENT_IDS.ANSWER_QUESTION, ({ draftDb }, i, ans) => {
draftDb.userAnswers[i] = ans;
return (appDb: Db) =>
[[EFFECT_IDS.LOCAL_STORAGE_SET, { key: 'userAnswers', value: appDb.userAnswers }]];
});Runtime:
- If payload is a function, run it after
produce/finishDraft, passing plainnewDb(orgetAppDb()if needed). Its return must be merged into the effect queue. - Draft-leak risk is eliminated (builder receives only plain data).
Pros:
- No
current()in user code. - Effects can be derived from the final state.
- No deep-finalization cost if all payloads are functions.
Cons:
- Two-phase mental model (build function now, execute later).
- Must ensure builders are pure and quick (no I/O on build).
7) Pass appDb into the effect handler at execution time
Idea: keep effects as data, but when executing an effect, the effect runner passes the current plain appDb to the effect handler, so payloads may be lightweight references/keys.
Usage (two styles):
a) Payload-as-function (lazy payload):
return [[
[EFFECT_IDS.LOCAL_STORAGE_SET, (appDb: Db) => ({ key: 'userAnswers', value: appDb.userAnswers })],
]
];Effect runner:
for (const [id, payload] of effects) {
const finalPayload = typeof payload === 'function' ? payload(appDb) : payload;
runEffect(id, finalPayload, { appDb });
}b) Provide appDb to effect handlers explicitly:
type EffectRunner = (payload: unknown, ctx: { appDb: Db; /* ... */ }) => Promise<void> | void;Pros:
- Zero draft leakage by design (execution sees only plain
appDb). - Powerful: effects can compute final payloads from the latest state.
Cons:
- If payloads are functions, you need to document purity/serializability expectations (they can’t cross worker boundaries, etc.).
- Slightly more complex effect runtime.
Notes on compatibility
finalizeEffectsshould skip functions (treat as opaque). Draft safety then relies on the deferred execution model.- Both options can coexist with the earlier auto-finalization. Provide a feature flag:
createStore({
autoFinalizeEffects: true, // deep unwrap drafts (default)
allowDeferredEffects: true // enable function payloads / builders
});