Skip to content

Commit 3fc3965

Browse files
authored
fix(core): prevent divergence with null map values (#18)
* fix(core): prevent divergence with null map values - diff: treat null as a present value when detecting added keys; guard container insertions with actual value type; fall back to value-based inference when schema/ type mismatches occur and warn to avoid divergence. - mirror: gate consistency check by checkStateConsistency (not debug); include raw doc.toJSON() in divergence log for easier debugging. - state: add checkStateConsistency option to createStore and plumb through to Mirror. - test(core): add null-in-map consistency tests and stabilize existing state tests with explicit commits. - chore(deps): bump loro-crdt to ^1.7.0 across core/react/jotai and update lockfile. * chore: fix lint issues
1 parent 5d1ce46 commit 3fc3965

File tree

11 files changed

+134
-198
lines changed

11 files changed

+134
-198
lines changed

IMPLEMENTATION_PLAN.md

Lines changed: 0 additions & 161 deletions
This file was deleted.

packages/core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,4 @@
3636
"@types/node": "^20.10.5",
3737
"typescript": "^5.3.3"
3838
}
39-
}
39+
}

packages/core/src/core/diff.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,15 +1011,31 @@ export function diffMap<S extends ObjectLike>(
10111011
if (childSchema && childSchema.type === "ignore") {
10121012
continue;
10131013
}
1014-
const containerType =
1014+
1015+
// Determine container type with schema-first, but respect actual value.
1016+
// If schema suggests a container but the provided value doesn't match it,
1017+
// log a warning and fall back to inferring from the value to avoid divergence.
1018+
let containerType =
10151019
childSchema?.getContainerType() ??
10161020
tryInferContainerType(newItem, inferOptions);
1021+
if (
1022+
childSchema?.getContainerType() &&
1023+
containerType &&
1024+
!isValueOfContainerType(containerType, newItem)
1025+
) {
1026+
console.warn(
1027+
`Schema mismatch on key "${key}": expected ${childSchema.getContainerType()} but got value ${JSON.stringify(
1028+
newItem,
1029+
)}. Falling back to value-based inference to avoid divergence.`,
1030+
);
1031+
containerType = tryInferContainerType(newItem, inferOptions);
1032+
}
10171033

10181034
// Added new key: detect by property presence, not truthiness.
10191035
// Using `!oldItem` breaks for valid falsy values like "" or null.
10201036
if (!(key in oldStateObj)) {
10211037
// Inserted a new container
1022-
if (containerType) {
1038+
if (containerType && isValueOfContainerType(containerType, newItem)) {
10231039
changes.push({
10241040
container: containerId,
10251041
key,

packages/core/src/core/loroEventApply.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
TreeID,
1010
} from "loro-crdt";
1111
import { isTreeID } from "./utils";
12-
import { CID_KEY } from "../constants";
1312

1413
// Plain JSON-like value held in Mirror state (no `any`)
1514
type JSONPrimitive = string | number | boolean | null | undefined;

packages/core/src/core/mirror.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,19 @@ export class Mirror<S extends SchemaType> {
302302

303303
// Subscribe to the root doc for global updates
304304
this.subscriptions.push(this.doc.subscribe(this.handleLoroEvent));
305+
306+
// If the caller provided an initial state, apply it via setState so
307+
// Loro is seeded consistently using the regular diff/apply pipeline.
308+
if (this.options.initialState && Object.keys(this.options.initialState).length > 0) {
309+
// Shallow apply the provided initial state
310+
const prevCheck = this.options.checkStateConsistency;
311+
this.options.checkStateConsistency = false;
312+
try {
313+
this.setState(this.options.initialState as Partial<InferType<S>>);
314+
} finally {
315+
this.options.checkStateConsistency = prevCheck;
316+
}
317+
}
305318
}
306319

307320
/**
@@ -350,7 +363,7 @@ export class Mirror<S extends SchemaType> {
350363
}
351364
}
352365

353-
// Build initial state snapshot
366+
// Build initial state snapshot from the current document
354367
const currentDocState = this.buildRootStateSnapshot();
355368
const newState = produce<InferType<S>>((draft) => {
356369
Object.assign(draft, currentDocState);
@@ -1805,12 +1818,10 @@ export class Mirror<S extends SchemaType> {
18051818
containerType === "List" ||
18061819
containerType === "MovableList"
18071820
) {
1808-
const l = container as unknown as LoroList | LoroMovableList;
1809-
if (l.length === 0) continue;
1821+
// Always include lists, even if empty, to match Mirror's state shape
18101822
root[key] = this.containerToStateJson(container);
18111823
} else if (containerType === "Text") {
1812-
const t = container as LoroText;
1813-
if ((t.toJSON() as string) === "") continue;
1824+
// Always include text, even if empty, to match Mirror's state shape
18141825
root[key] = this.containerToStateJson(container);
18151826
} else if (containerType === "Tree") {
18161827
const arr = this.containerToStateJson(container) as any[];

packages/core/src/core/state.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ export interface CreateStoreOptions<S extends SchemaType> {
4242
* @default false
4343
*/
4444
debug?: boolean;
45+
46+
checkStateConsistency?: boolean;
4547
}
4648

4749
/**
@@ -89,6 +91,7 @@ export function createStore<S extends SchemaType>(
8991
validateUpdates: options.validateUpdates,
9092
throwOnValidationError: options.throwOnValidationError ?? true,
9193
debug: options.debug,
94+
checkStateConsistency: options.checkStateConsistency,
9295
});
9396

9497
return {
@@ -108,14 +111,12 @@ export function createStore<S extends SchemaType>(
108111
export function createReducer<
109112
S extends SchemaType,
110113
A extends Record<string, unknown>,
111-
>(
112-
actionHandlers: {
113-
[K in keyof A]: (
114-
state: import("immer").Draft<InferType<S>>,
115-
payload: A[K],
116-
) => void;
117-
},
118-
) {
114+
>(actionHandlers: {
115+
[K in keyof A]: (
116+
state: import("immer").Draft<InferType<S>>,
117+
payload: A[K],
118+
) => void;
119+
}) {
119120
return (store: Store<S>) => {
120121
// Return a dispatch function that takes an action and payload
121122
return <K extends keyof A>(actionType: K, payload: A[K]) => {
@@ -125,11 +126,12 @@ export function createReducer<
125126
}
126127

127128
store.setState((state) =>
128-
produce<InferType<S>>(state, (
129-
draft: import("immer").Draft<InferType<S>>,
130-
) => {
131-
handler(draft, payload);
132-
}),
129+
produce<InferType<S>>(
130+
state,
131+
(draft: import("immer").Draft<InferType<S>>) => {
132+
handler(draft, payload);
133+
},
134+
),
133135
);
134136
};
135137
};

packages/core/tests/core/__snapshots__/mirror-movable-list.test.ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ exports[`MovableList > movable list handles basic sets 1`] = `
77
"deps": [],
88
"id": "0@0",
99
"lamport": 0,
10-
"msg": undefined,
10+
"msg": null,
1111
"ops": [
1212
{
1313
"container": "cid:root-list:MovableList",
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { describe, it, expect } from "vitest";
2+
import { LoroDoc, LoroMap, LoroList } from "loro-crdt";
3+
import { Mirror, schema, toNormalizedJson } from "../../src";
4+
5+
describe("setState consistency with null fields in LoroMap", () => {
6+
it("does not diverge when a loro-map field contains null and checkStateConsistency is enabled", async () => {
7+
const withSchema = schema({
8+
m: schema.LoroMap({
9+
nested: schema.LoroMap({}),
10+
}),
11+
});
12+
13+
const doc = new LoroDoc();
14+
const m = doc.getMap("m");
15+
// Pre-populate a null field inside a loro map
16+
m.set("nested", null);
17+
doc.commit();
18+
await Promise.resolve();
19+
20+
const mirror = new Mirror({
21+
schema: withSchema,
22+
doc,
23+
// rely on doc state; just enable consistency check
24+
checkStateConsistency: true,
25+
});
26+
27+
// Sanity: mirror picks up the null from doc
28+
expect(mirror.getState()).toEqual(toNormalizedJson(doc));
29+
console.log(JSON.stringify(doc.toJSON(), null, 2));
30+
31+
// Update another field (unrelated) to force a diff run
32+
expect(() => {
33+
mirror.setState((s) => {
34+
// write a new primitive field alongside nested
35+
(s as any).m["other"] = 1;
36+
});
37+
}).not.toThrow();
38+
39+
// State remains in sync with doc
40+
expect(mirror.getState()).toEqual(toNormalizedJson(doc));
41+
// And the original null is preserved
42+
expect((mirror.getState() as any).m.nested).toBeNull();
43+
});
44+
45+
it("remains stable when the null lives deeper (list -> map) and we no-op setState", async () => {
46+
const withSchema = schema({
47+
root: schema.LoroMap({
48+
list: schema.LoroList(
49+
schema.LoroMap({ child: schema.LoroMap({}) }),
50+
),
51+
}),
52+
});
53+
54+
const doc = new LoroDoc();
55+
const root = doc.getMap("root");
56+
// Attach a new list container under the root map, not the root-level list
57+
// to avoid introducing an unexpected top-level key for validation.
58+
const list = root.setContainer("list", new LoroList());
59+
const item = list.pushContainer(new LoroMap());
60+
// child is a loro-map by schema, but currently null in the doc JSON
61+
item.set("child", null);
62+
63+
const mirror = new Mirror({
64+
schema: withSchema,
65+
doc,
66+
checkStateConsistency: true,
67+
});
68+
69+
// No-op update should not throw or change representation
70+
expect(() => {
71+
mirror.setState((s) => s);
72+
}).not.toThrow();
73+
expect(mirror.getState()).toEqual(toNormalizedJson(doc));
74+
expect((mirror.getState() as any).root.list[0].child).toBeNull();
75+
});
76+
});

0 commit comments

Comments
 (0)