feat(tree): adds a changed event on TreeBranchEvents (microsoft#2…

…2977)

adds a new `changed` event to `TreeBranchEvents` that fires for both
local and remote changes

---------

Co-authored-by: Noah Encke <[email protected]>

.changeset/stinky-cars-fart.md

@@ -0,0 +1,11 @@
+---
+"@fluidframework/tree": minor
+---
+---
+"section": tree
+---
+
+Provide more comprehensive replacement to the `commitApplied` event
+
+Adds a new `changed` event to the (currently alpha) `TreeBranchEvents` that replaces the `commitApplied` event on `TreeViewEvents`.
+This new event is fired for both local and remote changes and maintains the existing functionality of `commitApplied` that is used for obtaining `Revertibles`.

packages/dds/tree/api-report/tree.alpha.api.md

@@ -725,6 +725,7 @@ export interface TreeBranch extends IDisposable {
 
 // @alpha @sealed
 export interface TreeBranchEvents {
+    changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
     commitApplied(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
     schemaChanged(): void;
 }
@@ -873,7 +874,9 @@ export interface TreeView<in out TSchema extends ImplicitFieldSchema> extends ID
 }
 
 // @alpha @sealed
-export interface TreeViewAlpha<in out TSchema extends ImplicitFieldSchema | UnsafeUnknownSchema> extends Omit<TreeView<ReadSchema<TSchema>>, "root" | "initialize">, Omit<TreeBranch, "events"> {
+export interface TreeViewAlpha<in out TSchema extends ImplicitFieldSchema | UnsafeUnknownSchema> extends Omit<TreeView<ReadSchema<TSchema>>, "root" | "initialize">, TreeBranch {
+    // (undocumented)
+    readonly events: Listenable<TreeViewEvents & TreeBranchEvents>;
     // (undocumented)
     fork(): ReturnType<TreeBranch["fork"]> & TreeViewAlpha<TSchema>;
     // (undocumented)

packages/dds/tree/src/core/rebase/types.ts

@@ -161,11 +161,11 @@ export interface GraphCommit<TChange> {
  * @public
  */
 export enum CommitKind {
-	/** A commit corresponding to a change that is not the result of an undo/redo. */
+	/** A commit corresponding to a change that is not the result of an undo/redo from this client. */
 	Default,
-	/** A commit that is the result of an undo. */
+	/** A commit that is the result of an undo from this client. */
 	Undo,
-	/** A commit that is the result of a redo. */
+	/** A commit that is the result of a redo from this client. */
 	Redo,
 }
 

packages/dds/tree/src/core/revertible.ts

@@ -50,7 +50,7 @@ export enum RevertibleStatus {
 
 /**
  * Factory for creating a {@link Revertible}.
- * Will error if invoked outside the scope of the `commitApplied` event that provides it, or if invoked multiple times.
+ * Will error if invoked outside the scope of the `changed` event that provides it, or if invoked multiple times.
  *
  * @param onRevertibleDisposed - A callback that will be invoked when the `Revertible` generated by this factory is disposed.
  * This happens when the `Revertible` is disposed manually, or when the `TreeView` that the `Revertible` belongs to is disposed,

packages/dds/tree/src/shared-tree/schematizingTreeView.ts

@@ -47,6 +47,7 @@ import {
 	type ReadSchema,
 	type UnsafeUnknownSchema,
 	type TreeBranch,
+	type TreeBranchEvents,
 } from "../simple-tree/index.js";
 import { Breakable, breakingClass, disposeSymbol, type WithBreakable } from "../util/index.js";
 
@@ -84,9 +85,9 @@ export class SchematizingSimpleTreeView<
 	 */
 	private currentCompatibility: SchemaCompatibilityStatus | undefined;
 	private readonly schemaPolicy: SchemaPolicy;
-	public readonly events: Listenable<TreeViewEvents> &
-		IEmitter<TreeViewEvents> &
-		HasListeners<TreeViewEvents> = createEmitter();
+	public readonly events: Listenable<TreeViewEvents & TreeBranchEvents> &
+		IEmitter<TreeViewEvents & TreeBranchEvents> &
+		HasListeners<TreeViewEvents & TreeBranchEvents> = createEmitter();
 
 	private readonly viewSchema: ViewSchema;
 
@@ -133,9 +134,10 @@ export class SchematizingSimpleTreeView<
 		this.update();
 
 		this.unregisterCallbacks.add(
-			this.checkout.events.on("commitApplied", (data, getRevertible) =>
-				this.events.emit("commitApplied", data, getRevertible),
-			),
+			this.checkout.events.on("changed", (data, getRevertible) => {
+				this.events.emit("changed", data, getRevertible);
+				this.events.emit("commitApplied", data, getRevertible);
+			}),
 		);
 	}
 

packages/dds/tree/src/shared-tree/treeCheckout.ts

@@ -85,18 +85,14 @@ export interface CheckoutEvents {
 	afterBatch(): void;
 
 	/**
-	 * Fired when a revertible change has been made to this view.
+	 * Fired when a change is made to the branch. Includes data about the change that is made which listeners
+	 * can use to filter on changes they care about e.g. local vs remote changes.
 	 *
-	 * Applications which subscribe to this event are expected to revert or discard revertibles they acquire (failure to do so will leak memory).
-	 * The provided revertible is inherently bound to the view that raised the event, calling `revert` won't apply to forked views.
-	 *
-	 * @param revertible - The revertible that can be used to revert the change.
-	 */
-
-	/**
-	 * {@inheritdoc TreeViewEvents.commitApplied}
+	 * @param data - information about the change
+	 * @param getRevertible - a function provided that allows users to get a revertible for the change. If not provided,
+	 * this change is not revertible.
 	 */
-	commitApplied(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+	changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
 }
 
 /**
@@ -118,7 +114,7 @@ export interface BranchableTree extends ViewableTree {
 	 * @param view - a branch which was created by a call to `branch()`.
 	 * It is automatically disposed after the merge completes.
 	 * @remarks All ongoing transactions (if any) in `branch` will be committed before the merge.
-	 * A "commitApplied" event and a corresponding {@link Revertible} will be emitted on this branch for each new change merged from 'branch'.
+	 * A "changed" event and a corresponding {@link Revertible} will be emitted on this branch for each new change merged from 'branch'.
 	 */
 	merge(branch: TreeBranchFork): void;
 
@@ -532,12 +528,12 @@ export class TreeCheckout implements ITreeCheckoutFork {
 							: (onRevertibleDisposed?: (revertible: Revertible) => void) => {
 									if (!withinEventContext) {
 										throw new UsageError(
-											"Cannot get a revertible outside of the context of a commitApplied event.",
+											"Cannot get a revertible outside of the context of a changed event.",
 										);
 									}
 									if (this.revertibleCommitBranches.get(revision) !== undefined) {
 										throw new UsageError(
-											"Cannot generate the same revertible more than once. Note that this can happen when multiple commitApplied event listeners are registered.",
+											"Cannot generate the same revertible more than once. Note that this can happen when multiple changed event listeners are registered.",
 										);
 									}
 									const revertibleCommits = this.revertibleCommitBranches;
@@ -582,9 +578,12 @@ export class TreeCheckout implements ITreeCheckoutFork {
 								};
 
 						let withinEventContext = true;
-						this.events.emit("commitApplied", { isLocal: true, kind }, getRevertible);
+						this.events.emit("changed", { isLocal: true, kind }, getRevertible);
 						withinEventContext = false;
 					}
+				} else if (event.type === "replace") {
+					// TODO: figure out how to plumb through commit kind info for remote changes
+					this.events.emit("changed", { isLocal: false, kind: CommitKind.Default });
 				}
 			}
 		});

packages/dds/tree/src/simple-tree/api/tree.ts

@@ -522,11 +522,13 @@ export interface TreeView<in out TSchema extends ImplicitFieldSchema> extends ID
 export interface TreeViewAlpha<
 	in out TSchema extends ImplicitFieldSchema | UnsafeUnknownSchema,
 > extends Omit<TreeView<ReadSchema<TSchema>>, "root" | "initialize">,
-		Omit<TreeBranch, "events"> {
+		TreeBranch {
 	get root(): ReadableField<TSchema>;
 
 	set root(newRoot: InsertableField<TSchema>);
 
+	readonly events: Listenable<TreeViewEvents & TreeBranchEvents>;
+
 	initialize(content: InsertableField<TSchema>): void;
 
 	// Override the base branch method to return a typed view rather than merely a branch.
@@ -619,6 +621,16 @@ export interface TreeBranchEvents {
 	 */
 	schemaChanged(): void;
 
+	/**
+	 * Fired when a change is made to the branch. Includes data about the change that is made which listeners
+	 * can use to filter on changes they care about (e.g. local vs. remote changes).
+	 *
+	 * @param data - information about the change
+	 * @param getRevertible - a function that allows users to get a revertible for the change. If not provided,
+	 * this change is not revertible.
+	 */
+	changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+
 	/**
 	 * Fired when:
 	 * - a local commit is applied outside of a transaction

packages/dds/tree/src/test/shared-tree/fuzz/undoRedo.fuzz.spec.ts

@@ -225,7 +225,7 @@ function init(state: UndoRedoFuzzTestState) {
 	state.unsubscribe = [];
 	for (const client of state.clients) {
 		const checkout = viewFromState(state, client).checkout;
-		const unsubscribe = checkout.events.on("commitApplied", (commit, getRevertible) => {
+		const unsubscribe = checkout.events.on("changed", (commit, getRevertible) => {
 			if (getRevertible !== undefined) {
 				if (commit.kind === CommitKind.Undo) {
 					redoStack.push(getRevertible());

packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts

@@ -64,7 +64,7 @@ import {
 	SchemaFactory,
 	toStoredSchema,
 	type TreeFieldFromImplicitField,
-	type TreeView,
+	type TreeViewAlpha,
 	TreeViewConfiguration,
 } from "../../simple-tree/index.js";
 import { brand, fail } from "../../util/index.js";
@@ -832,6 +832,83 @@ describe("SharedTree", () => {
 	});
 
 	describe("Undo and redo", () => {
+		it("the insert of a node in a sequence field using the commitApplied event", () => {
+			const value = "42";
+			const provider = new TestTreeProviderLite(2);
+			const tree1 = provider.trees[0];
+			const view1 = tree1.viewWith(
+				new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
+			);
+			view1.initialize([]);
+
+			const undoStack: Revertible[] = [];
+			const redoStack: Revertible[] = [];
+
+			function onDispose(disposed: Revertible): void {
+				const redoIndex = redoStack.indexOf(disposed);
+				if (redoIndex !== -1) {
+					redoStack.splice(redoIndex, 1);
+				} else {
+					const undoIndex = undoStack.indexOf(disposed);
+					if (undoIndex !== -1) {
+						undoStack.splice(undoIndex, 1);
+					}
+				}
+			}
+
+			const unsubscribeFromCommitAppliedEvent = view1.events.on(
+				"commitApplied",
+				(commit, getRevertible) => {
+					if (getRevertible !== undefined) {
+						const revertible = getRevertible(onDispose);
+						if (commit.kind === CommitKind.Undo) {
+							redoStack.push(revertible);
+						} else {
+							undoStack.push(revertible);
+						}
+					}
+				},
+			);
+			const unsubscribe = (): void => {
+				unsubscribeFromCommitAppliedEvent();
+				for (const revertible of undoStack) {
+					revertible.dispose();
+				}
+				for (const revertible of redoStack) {
+					revertible.dispose();
+				}
+			};
+
+			provider.processMessages();
+			const tree2 = provider.trees[1];
+			const view2 = tree2.viewWith(
+				new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
+			);
+			provider.processMessages();
+
+			// Insert node
+			view1.root.insertAtStart(value);
+			provider.processMessages();
+
+			// Validate insertion
+			assert.deepEqual([...view2.root], [value]);
+
+			// Undo node insertion
+			undoStack.pop()?.revert();
+			provider.processMessages();
+
+			assert.deepEqual([...view1.root], []);
+			assert.deepEqual([...view2.root], []);
+
+			// Redo node insertion
+			redoStack.pop()?.revert();
+			provider.processMessages();
+
+			assert.deepEqual([...view1.root], [value]);
+			assert.deepEqual([...view2.root], [value]);
+			unsubscribe();
+		});
+
 		it("the insert of a node in a sequence field", () => {
 			const value = "42";
 			const provider = new TestTreeProviderLite(2);
@@ -1159,7 +1236,7 @@ describe("SharedTree", () => {
 
 			interface Peer extends ConnectionSetter {
 				readonly checkout: TreeCheckout;
-				readonly view: TreeView<typeof schema>;
+				readonly view: TreeViewAlpha<typeof schema>;
 				readonly outerList: TreeFieldFromImplicitField<typeof schema>;
 				readonly innerList: TreeFieldFromImplicitField<typeof innerListSchema>;
 				assertOuterListEquals(expected: readonly (readonly string[])[]): void;
@@ -1168,7 +1245,7 @@ describe("SharedTree", () => {
 
 			function makeUndoableEdit(peer: Peer, edit: () => void): Revertible {
 				const undos: Revertible[] = [];
-				const unsubscribe = peer.view.events.on("commitApplied", ({ kind }, getRevertible) => {
+				const unsubscribe = peer.view.events.on("changed", ({ kind }, getRevertible) => {
 					if (kind !== CommitKind.Undo && getRevertible !== undefined) {
 						undos.push(getRevertible());
 					}
@@ -1474,6 +1551,76 @@ describe("SharedTree", () => {
 			unsubscribe1();
 			unsubscribe2();
 		});
+
+		// TODO: move this event test to the tree view tests
+		it("emits a changed event for local edits", () => {
+			const value = "42";
+			const provider = new TestTreeProviderLite(2);
+			const tree1 = provider.trees[0];
+			const view1 = tree1.viewWith(
+				new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
+			);
+			view1.initialize([]);
+			provider.processMessages();
+
+			let localEdits = 0;
+			let remoteEdits = 0;
+
+			const unsubscribe = view1.events.on("changed", (metadata) => {
+				if (metadata.isLocal === true) {
+					localEdits++;
+				} else {
+					remoteEdits++;
+				}
+			});
+
+			// Insert node
+			view1.root.insertAtStart(value);
+			provider.processMessages();
+
+			// Validate insertion
+			assert.deepEqual([...view1.root], [value]);
+
+			assert.equal(localEdits, 1);
+			// check that the edit is not counted twice
+			assert.equal(remoteEdits, 0);
+
+			unsubscribe();
+		});
+
+		it("emits a changed event for remote edits", () => {
+			const value = "42";
+			const provider = new TestTreeProviderLite(2);
+			const tree1 = provider.trees[0];
+			const view1 = tree1.viewWith(
+				new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
+			);
+			view1.initialize([]);
+			provider.processMessages();
+			const tree2 = provider.trees[1];
+			const view2 = tree2.viewWith(
+				new TreeViewConfiguration({ schema: StringArray, enableSchemaValidation }),
+			);
+
+			let remoteEdits = 0;
+
+			const unsubscribe = view1.events.on("changed", (metadata) => {
+				if (metadata.isLocal !== true) {
+					remoteEdits++;
+				}
+			});
+
+			// Insert node
+			view2.root.insertAtStart(value);
+			provider.processMessages();
+
+			// Validate insertion
+			assert.deepEqual([...view1.root], [value]);
+
+			assert.equal(remoteEdits, 1);
+
+			unsubscribe();
+		});
 	});
 
 	describe("Rebasing", () => {