Skip to content

Commit

Permalink
tree: Track which fields change in anchorSet (#22367)
Browse files Browse the repository at this point in the history
## Description

Split off from #22229
for a smaller review.
  • Loading branch information
CraigMacomber authored Sep 3, 2024
1 parent f4da8fc commit 04849aa
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 11 deletions.
44 changes: 35 additions & 9 deletions packages/dds/tree/src/core/tree/anchorSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import type {
RangeUpPath,
UpPath,
} from "./pathTree.js";
import { EmptyKey, type Value } from "./types.js";
import { EmptyKey } from "./types.js";
import type { DeltaVisitor } from "./visitDelta.js";
import type { PathVisitor } from "./visitPath.js";
import type { AnnouncedVisitor } from "./visitorUtils.js";
Expand Down Expand Up @@ -127,7 +127,10 @@ export interface AnchorEvents {
*
* Compare to {@link AnchorEvents.childrenChanged} which is emitted in the middle of the batch/delta-visit.
*/
childrenChangedAfterBatch(anchor: AnchorNode): void;
childrenChangedAfterBatch(arg: {
anchor: AnchorNode;
changedFields: ReadonlySet<FieldKey>;
}): void;

/**
* Emitted in the middle of applying a batch of changes (i.e. during a delta a visit), if something in the subtree
Expand Down Expand Up @@ -177,11 +180,6 @@ export interface AnchorEvents {
* fire when something _may_ have changed or _may_ be about to change.
*/
subtreeChangedAfterBatch(anchor: AnchorNode): void;

/**
* Value on this node is changing.
*/
valueChanging(anchor: AnchorNode, value: Value): void;
}

/**
Expand Down Expand Up @@ -715,7 +713,19 @@ export class AnchorSet implements Listenable<AnchorSetRootEvents>, AnchorLocator
pathVisitors: new Map<PathNode, Set<PathVisitor>>(),
parentField: undefined as FieldKey | undefined,
parent: undefined as UpPath | undefined,
bufferedEvents: [] as { node: PathNode; event: keyof AnchorEvents }[],

/**
* Events collected during the visit which get sent as a batch during "free".
*/
bufferedEvents: [] as {
node: PathNode;
event: keyof AnchorEvents;
/**
* The key for the impacted field, if the event is associated with a key.
* Some events, such as afterDestroy, do not involve a key, and thus leave this undefined.
*/
changedField?: FieldKey;
}[],

// 'currentDepth' and 'depthThresholdForSubtreeChanged' serve to keep track of when do we need to emit
// subtreeChangedAfterBatch events.
Expand Down Expand Up @@ -757,7 +767,18 @@ export class AnchorSet implements Listenable<AnchorSetRootEvents>, AnchorLocator
continue;
}
emittedEvents?.push(event);
node.events.emit(event, node);
if (event === "childrenChangedAfterBatch") {
const fieldKeys: FieldKey[] = this.bufferedEvents
.filter((e) => e.node === node && e.event === event)
.map(
(e) =>
e.changedField ??
fail("childrenChangedAfterBatch events should have a changedField"),
);
node.events.emit(event, { anchor: node, changedFields: new Set(fieldKeys) });
} else {
node.events.emit(event, node);
}
}
},
notifyChildrenChanging(): void {
Expand All @@ -769,10 +790,15 @@ export class AnchorSet implements Listenable<AnchorSetRootEvents>, AnchorLocator
notifyChildrenChanged(): void {
this.maybeWithNode(
(p) => {
assert(
this.parentField !== undefined,
"Must be in a field to modify its contents",
);
p.events.emit("childrenChanged", p);
this.bufferedEvents.push({
node: p,
event: "childrenChangedAfterBatch",
changedField: this.parentField,
});
},
() => {},
Expand Down
9 changes: 7 additions & 2 deletions packages/dds/tree/src/core/tree/visitDelta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,13 @@ function transferRoots(
*/
export interface DeltaVisitor {
/**
* Frees/releases the visitor. Must be called once the visitor is no longer needed, since trying to acquire
* a new one before freeing an existing one is invalid.
* Frees/releases the visitor.
*
* Must be called once the visitor finished traversing the delta for a couple of reasons:
*
* 1. Some visitors, such as those from forests, are put into a special mode while they have a visitor, forbidding some actions (like making more visitors).
*
* 2. Some visitors, such as those from an anchorSet, defer some events for batching purposes until the visitor is freed.
*/
free(): void;
/**
Expand Down
43 changes: 43 additions & 0 deletions packages/dds/tree/src/test/tree/anchorSet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,49 @@ describe("AnchorSet", () => {
]);
});

it("childrenChangedAfterBatch event includes the changed fields", () => {
const fieldOne: FieldKey = brand("one");
const fieldTwo: FieldKey = brand("two");
const fieldThree: FieldKey = brand("three");

const anchors = new AnchorSet();

const anchor0 = anchors.track(makePath([rootFieldKey, 0]));
const node0 = anchors.locate(anchor0) ?? assert.fail();

const expectedChangedFields = new Set<FieldKey>([fieldOne, fieldTwo, fieldThree]);
let listenerFired = false;
node0.on("childrenChangedAfterBatch", ({ anchor, changedFields }) => {
// This is the main validation of this test
assert.deepEqual(changedFields, expectedChangedFields);
listenerFired = true;
});

// Try to test all cases of changes happening on a delta visitor: attaches, detaches, replaces
withVisitor(anchors, (v) => {
v.enterField(rootFieldKey);
v.enterNode(0);
v.enterField(fieldOne);
v.detach({ start: 0, end: 1 }, brand("fakeDetachDestination"));
v.exitField(fieldOne);
v.enterField(fieldTwo);
v.attach(brand("fakeAttachSource"), 1, 0);
v.exitField(fieldTwo);
v.enterField(fieldThree);
v.replace(
brand("fakeReplaceSource"),
{ start: 0, end: 1 },
brand("fakeReplaceDestination"),
);
v.exitField(fieldThree);
v.exitNode(0);
v.exitField(rootFieldKey);
});

// Make sure the listener actually fired and validated the changed fields.
assert.equal(listenerFired, true);
});

it("triggers path visitor callbacks", () => {
const build = [
{
Expand Down

0 comments on commit 04849aa

Please sign in to comment.