Skip to content

Remove unneeded anchor events #24474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Remove unneeded anchor events #24474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 1 addition & 64 deletions packages/dds/tree/src/core/tree/anchorSet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,6 @@ export interface AnchorEvents {
*/
childrenChanging(anchor: AnchorNode): void;

/**
* Emitted in the middle of applying a batch of changes (i.e. during a delta a visit), if one or more of this node's
* direct children just changed due to updates from the batch.
*
* @remarks
* Does not include edits of child subtrees: instead only includes changes to nodes which are direct children in this
* node's fields.
*
* Compare to {@link AnchorEvents.childrenChangedAfterBatch} which is emitted after the whole batch has been applied.
*/
childrenChanged(anchor: AnchorNode): void;

/**
* Emitted after a batch of changes has been applied (i.e. when a delta visit completes), if one or more of this node's
* direct children changed due to updates from the batch.
Expand All @@ -132,36 +120,6 @@ export interface AnchorEvents {
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
* rooted at `anchor` _may_ be about to change due to updates from the batch.
*
* @remarks
* Called on every parent (transitively) when a change is occurring.
*/
subtreeChanging(anchor: AnchorNode): void;

/**
* Emitted in the middle of applying a batch of changes (i.e. during a delta a visit), if something in the subtree
* rooted at `anchor` _may_ have just changed due to updates from the batch.
*
* @remarks
* While this event is always emitted in the presence of changes to the subtree,
* it may also be emitted even though no changes have been made to the subtree.
* It may be emitted multiple times within the application of a single edit or transaction.
*
* If this event is emitted by a node, it will later be emitted by all its ancestors up to the root as well, at
* least once on each ancestor.
*
* Compare to {@link AnchorEvents.subtreeChangedAfterBatch} which is emitted after the whole batch has been applied.
*
* @privateRemarks
* The delta visit algorithm is complicated and it may fire this event multiple times for the same change to a node.
* The change to the tree may not be visible until the event fires for the last time.
* Refer to the documentation of the delta visit algorithm for more details.
*/
subtreeChanged(anchor: AnchorNode): void;

/**
* Emitted after a batch of changes has been applied (i.e. when a delta visit completes), if something in the subtree
* rooted at `anchor` changed due to updates from the batch.
Expand All @@ -170,13 +128,6 @@ export interface AnchorEvents {
* If this event is emitted by a node, it will later be emitted by all its ancestors up to the root as well, from bottom to top.
*
* This event is guaranteed to be emitted on a given node only once per batch.
*
* Compare to {@link AnchorEvents.subtreeChanged} which is emitted in the middle of the batch/delta-visit.
*
* @privateRemarks
* Note that because this is fired after the full batch of changes is applied, it guarantees that something in the
* subtree changed, compared to {@link AnchorEvents.subtreeChanged} or {@link AnchorEvents.subtreeChanging} which
* fire when something _may_ have changed or _may_ be about to change.
*/
subtreeChangedAfterBatch(): void;
}
Expand All @@ -192,11 +143,6 @@ export interface AnchorEvents {
* - Add more events.
*/
export interface AnchorSetRootEvents {
/**
* What children are at the root is changing.
*/
childrenChanging(anchors: AnchorSet): void;

/**
* Something in the tree is changing.
*/
Expand Down Expand Up @@ -798,10 +744,7 @@ export class AnchorSet implements AnchorLocator {
}
},
notifyChildrenChanging(): void {
this.maybeWithNode(
(p) => p.events.emit("childrenChanging", p),
() => this.anchorSet.#events.emit("childrenChanging", this.anchorSet),
);
this.maybeWithNode((p) => p.events.emit("childrenChanging", p));
},
notifyChildrenChanged(): void {
this.maybeWithNode(
Expand All @@ -810,7 +753,6 @@ export class AnchorSet implements AnchorLocator {
this.parentField !== undefined,
0xa24 /* Must be in a field to modify its contents */,
);
p.events.emit("childrenChanged", p);
this.bufferedEvents.push({
node: p,
event: "childrenChangedAfterBatch",
Expand Down Expand Up @@ -909,17 +851,12 @@ export class AnchorSet implements AnchorLocator {
parentIndex: index,
};
this.parentField = undefined;
this.maybeWithNode((p) => {
p.events.emit("subtreeChanging", p);
});
this.currentDepth++;
},
exitNode(index: number): void {
assert(this.parent !== undefined, 0x3ac /* Must have parent node */);
if (this.depthThresholdForSubtreeChanged === this.currentDepth) {
this.maybeWithNode((p) => {
p.events.emit("subtreeChanged", p);

this.bufferedEvents.push({
node: p,
event: "subtreeChangedAfterBatch",
Expand Down
20 changes: 0 additions & 20 deletions packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,32 +87,22 @@ describe("sharedTreeView", () => {
const anchorNode = getOrCreateInnerNode(root).anchorNode;
const log: string[] = [];
const unsubscribe = anchorNode.events.on("childrenChanging", () => log.push("change"));
const unsubscribeSubtree = anchorNode.events.on("subtreeChanging", () => {
log.push("subtree");
});
const unsubscribeAfter = view.checkout.events.on("afterBatch", () => log.push("after"));
log.push("editStart");
root.x = 5;
log.push("editStart");
root.x = 6;
log.push("unsubscribe");
unsubscribe();
unsubscribeSubtree();
unsubscribeAfter();
log.push("editStart");
root.x = 7;

assert.deepEqual(log, [
"editStart",
"subtree",
"change",
"subtree",
"change",
"after",
"editStart",
"subtree",
"change",
"subtree",
"change",
"after",
"unsubscribe",
Expand All @@ -131,32 +121,22 @@ describe("sharedTreeView", () => {
const unsubscribe = anchorNode.events.on("childrenChanging", (upPath) =>
log.push(`change-${String(upPath.parentField)}-${upPath.parentIndex}`),
);
const unsubscribeSubtree = anchorNode.events.on("subtreeChanging", (upPath) => {
log.push(`subtree-${String(upPath.parentField)}-${upPath.parentIndex}`);
});
const unsubscribeAfter = view.checkout.events.on("afterBatch", () => log.push("after"));
log.push("editStart");
root.x = 5;
log.push("editStart");
root.x = 6;
log.push("unsubscribe");
unsubscribe();
unsubscribeSubtree();
unsubscribeAfter();
log.push("editStart");
root.x = 7;

assert.deepEqual(log, [
"editStart",
"subtree-rootFieldKey-0",
"change-rootFieldKey-0",
"subtree-rootFieldKey-0",
"change-rootFieldKey-0",
"after",
"editStart",
"subtree-rootFieldKey-0",
"change-rootFieldKey-0",
"subtree-rootFieldKey-0",
"change-rootFieldKey-0",
"after",
"unsubscribe",
Expand Down
5 changes: 1 addition & 4 deletions packages/dds/tree/src/test/tree/anchorSet.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,11 +446,10 @@ describe("AnchorSet", () => {
});
});

it("triggers childrenChanging, childrenChanged, treeChanging, subtreeChanging, and afterDestroy callbacks", () => {
it("triggers childrenChanging, treeChanging, and afterDestroy callbacks", () => {
// AnchorSet does not guarantee event ordering within a batch so use UnorderedTestLogger.
const log = new UnorderedTestLogger();
const anchors = new AnchorSet();
anchors.events.on("childrenChanging", log.logger("root childrenChange"));
anchors.events.on("treeChanging", log.logger("root treeChange"));

const detachMark: DeltaMark = {
Expand All @@ -471,8 +470,6 @@ describe("AnchorSet", () => {
const node0 = anchors.locate(anchor0) ?? assert.fail();

node0.events.on("childrenChanging", log.logger("childrenChanging"));
node0.events.on("childrenChanged", log.logger("childrenChanged"));
node0.events.on("subtreeChanging", log.logger("subtreeChange"));
node0.events.on("afterDestroy", log.logger("afterDestroy"));

log.expect([]);
Expand Down
Loading