diff --git a/packages/dds/tree/src/simple-tree/arrayNode.ts b/packages/dds/tree/src/simple-tree/arrayNode.ts index af173e3985a9..1d52a05bde59 100644 --- a/packages/dds/tree/src/simple-tree/arrayNode.ts +++ b/packages/dds/tree/src/simple-tree/arrayNode.ts @@ -32,6 +32,7 @@ import { type TreeNodeSchema, typeNameSymbol, normalizeFieldSchema, + nodeKindSymbol, } from "./schemaTypes.js"; import { mapTreeFromNodeData } from "./toMapTree.js"; import { @@ -976,6 +977,10 @@ export function arraySchema< return identifier; } + public get [nodeKindSymbol](): NodeKind { + return NodeKind.Array; + } + protected get simpleSchema(): T { return info; } diff --git a/packages/dds/tree/src/simple-tree/mapNode.ts b/packages/dds/tree/src/simple-tree/mapNode.ts index eefeee10ef5f..bf4f323ac661 100644 --- a/packages/dds/tree/src/simple-tree/mapNode.ts +++ b/packages/dds/tree/src/simple-tree/mapNode.ts @@ -29,6 +29,7 @@ import { type TreeNodeSchema, type TreeNodeFromImplicitAllowedTypes, typeNameSymbol, + nodeKindSymbol, } from "./schemaTypes.js"; import { mapTreeFromNodeData } from "./toMapTree.js"; import { type MostDerivedData, type TreeNode, TreeNodeValid } from "./types.js"; @@ -257,6 +258,10 @@ export function mapSchema< public get [typeNameSymbol](): TName { return identifier; } + + public get [nodeKindSymbol](): NodeKind { + return NodeKind.Map; + } } const schemaErased: TreeNodeSchemaClass< TName, diff --git a/packages/dds/tree/src/simple-tree/objectNode.ts b/packages/dds/tree/src/simple-tree/objectNode.ts index 67a4a4ccad41..43fa340b5a9a 100644 --- a/packages/dds/tree/src/simple-tree/objectNode.ts +++ b/packages/dds/tree/src/simple-tree/objectNode.ts @@ -40,6 +40,7 @@ import { typeNameSymbol, type ImplicitAllowedTypes, FieldKind, + nodeKindSymbol, } from "./schemaTypes.js"; import { mapTreeFromNodeData } from "./toMapTree.js"; import { @@ -305,6 +306,10 @@ abstract class CustomObjectNodeBase< const T extends RestrictiveReadonlyRecord, > extends TreeNodeValid> { public static readonly kind = NodeKind.Object; + + public get [nodeKindSymbol](): NodeKind { + return NodeKind.Object; + } } /** diff --git a/packages/dds/tree/src/simple-tree/schemaCreationUtilities.ts b/packages/dds/tree/src/simple-tree/schemaCreationUtilities.ts index 180b7e3dfe9d..2274556f67c7 100644 --- a/packages/dds/tree/src/simple-tree/schemaCreationUtilities.ts +++ b/packages/dds/tree/src/simple-tree/schemaCreationUtilities.ts @@ -9,7 +9,12 @@ import type { EmptyObject } from "../feature-libraries/index.js"; import { fail } from "../util/index.js"; import type { SchemaFactory, ScopedSchemaName } from "./schemaFactory.js"; -import type { NodeFromSchema, NodeKind, TreeNodeSchemaClass } from "./schemaTypes.js"; +import { + NodeKind, + nodeKindSymbol, + type NodeFromSchema, + type TreeNodeSchemaClass, +} from "./schemaTypes.js"; import type { TreeNode } from "./types.js"; import type { InsertableObjectFromSchemaRecord, @@ -36,6 +41,10 @@ export function singletonSchema void; + /** + * Register an event listener on an array node. This is a specialized version of {@link on} for array nodes, with + * listener payload types specific to them. + * @param node - The node whose events should be subscribed to. + * @param eventName - Which event to subscribe to. + * @param listener - The callback to trigger for the event. The tree can be read during the callback, but it is invalid to modify the tree during this callback. + * @returns A callback function which will deregister the event. + * This callback should be called only once. + */ + on( + node: TreeArrayNode, + eventName: K, + listener: ArrayNodeChangeEvents[K], + ): () => void; + + /** + * Register an event listener on an object node. This is a specialized version of {@link on} for object nodes, with + * listener payload types specific to them. + * @param node - The node whose events should be subscribed to. + * @param eventName - Which event to subscribe to. + * @param listener - The callback to trigger for the event. The tree can be read during the callback, but it is invalid to modify the tree during this callback. + * @returns A callback function which will deregister the event. + * This callback should be called only once. + */ + on( + node: TreeObjectNode, + eventName: K, + listener: ObjectNodeChangeEvents[K], + ): () => void; + + /** + * Register an event listener on an map node. This is a specialized version of {@link on} for map nodes, with + * listener payload types specific to them. + * @param node - The node whose events should be subscribed to. + * @param eventName - Which event to subscribe to. + * @param listener - The callback to trigger for the event. The tree can be read during the callback, but it is invalid to modify the tree during this callback. + * @returns A callback function which will deregister the event. + * This callback should be called only once. + */ + on( + node: TreeMapNode, + eventName: K, + listener: MapNodeChangeEvents[K], + ): () => void; + /** * Returns the {@link TreeStatus} of the given node. */ diff --git a/packages/dds/tree/src/simple-tree/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/treeNodeKernel.ts index fe166fee05f6..53e01fc27dd7 100644 --- a/packages/dds/tree/src/simple-tree/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/treeNodeKernel.ts @@ -5,7 +5,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import { createEmitter, type Listenable, type Off } from "../events/index.js"; -import type { TreeChangeEvents, TreeNode } from "./types.js"; +import type { TreeChangeEvents, TreeChangeEventsInternal, TreeNode } from "./types.js"; import type { AnchorNode } from "../core/index.js"; import { flexTreeSlot, @@ -25,12 +25,12 @@ import { NodeKind } from "./schemaTypes.js"; * The kernel has the same lifetime as the node and spans both its unhydrated and hydrated states. * When hydration occurs, the kernel is notified via the {@link TreeNodeKernel.hydrate | hydrate} method. */ -export class TreeNodeKernel implements Listenable { +export class TreeNodeKernel implements Listenable { #hydrated?: { anchorNode: AnchorNode; offAnchorNode: Off; }; - #events = createEmitter(); + #events = createEmitter(); public constructor(public readonly node: TreeNode) {} @@ -107,9 +107,9 @@ export class TreeNodeKernel implements Listenable { return treeStatusFromAnchorCache(this.#hydrated.anchorNode); } - public on( + public on( eventName: K, - listener: TreeChangeEvents[K], + listener: TreeChangeEventsInternal[K], ): Off { return this.#events.on(eventName, listener); } diff --git a/packages/dds/tree/src/simple-tree/types.ts b/packages/dds/tree/src/simple-tree/types.ts index 497b1e0303e8..28026b714486 100644 --- a/packages/dds/tree/src/simple-tree/types.ts +++ b/packages/dds/tree/src/simple-tree/types.ts @@ -11,6 +11,7 @@ import { type TreeNodeSchema, type TreeNodeSchemaClass, type WithType, + type nodeKindSymbol, typeNameSymbol, } from "./schemaTypes.js"; import { @@ -38,6 +39,69 @@ import { tryGetSchema } from "./treeNodeApi.js"; */ export type Unhydrated = T; +export interface NodeChangedEventArgs { + readonly changedProperties: ReadonlySet; +} + +export interface TreeChangeEventsBase { + /** + * Emitted by a node after a batch of changes has been applied to the tree, when something changed anywhere in the + * subtree rooted at it. + * + * @remarks + * This event is not emitted when the node itself is moved to a different location in the tree or removed from the tree. + * In that case it is emitted on the _parent_ node, not the node itself. + * + * The node itself is part of the subtree, so this event will be emitted even if the only changes are to the properties + * of the node itself. + * + * For remote edits, this event is not guaranteed to occur in the same order or quantity that it did in + * the client that made the original edit. + * + * When it is emitted, the tree is guaranteed to be in-schema. + */ + treeChanged(): void; +} + +export interface TreeChangeEventsInternal extends TreeChangeEventsBase { + /** + * Emitted by a node after a batch of changes has been applied to the tree, if a change affected the node, where a + * change is: + * + * - For an object node, when the value of one of its properties changes (i.e., the property's value is set + * to something else, including `undefined`). + * + * - For an array node, when an element is added, removed, or moved. + * + * - For a map node, when an entry is added, updated, or removed. + * + * @remarks + * Prefer using events on the specific node kind (object nodes, map nodes, array nodes) instead of this one + * for more specific listener signatures according to what a given node kind actually supports. + * + * This event is not emitted when: + * + * - Properties of a child node change. Notably, updates to an array node or a map node (like adding or removing + * elements/entries) will emit this event on the array/map node itself, but not on the node that contains the + * array/map node as one of its properties. + * + * - The node is moved to a different location in the tree or removed from the tree. + * In this case the event is emitted on the _parent_ node, not the node itself. + * + * For remote edits, this event is not guaranteed to occur in the same order or quantity that it did in + * the client that made the original edit. + * + * When it is emitted, the tree is guaranteed to be in-schema. + * + * @privateRemarks + * This event occurs whenever the apparent contents of the node instance change, regardless of what caused the change. + * For example, it will fire when the local client reassigns a child, when part of a remote edit is applied to the + * node, or when the node has to be updated due to resolution of a merge conflict + * (for example a previously applied local change might be undone, then reapplied differently or not at all). + */ + nodeChanged(args: NodeChangedEventArgs): void; +} + /** * A collection of events that can be emitted by a {@link TreeNode}. * @@ -59,7 +123,7 @@ export type Unhydrated = T; * * @sealed @public */ -export interface TreeChangeEvents { +export interface TreeChangeEvents extends TreeChangeEventsBase { /** * Emitted by a node after a batch of changes has been applied to the tree, if a change affected the node, where a * change is: @@ -72,6 +136,39 @@ export interface TreeChangeEvents { * - For a map node, when an entry is added, updated, or removed. * * @remarks + * Prefer using events on the specific node kind (object nodes, map nodes, array nodes) instead of this one + * for more specific listener signatures according to what a given node kind actually supports. + * + * This event is not emitted when: + * + * - Properties of a child node change. Notably, updates to an array node or a map node (like adding or removing + * elements/entries) will emit this event on the array/map node itself, but not on the node that contains the + * array/map node as one of its properties. + * + * - The node is moved to a different location in the tree or removed from the tree. + * In this case the event is emitted on the _parent_ node, not the node itself. + * + * For remote edits, this event is not guaranteed to occur in the same order or quantity that it did in + * the client that made the original edit. + * + * When it is emitted, the tree is guaranteed to be in-schema. + * + * @privateRemarks + * This event occurs whenever the apparent contents of the node instance change, regardless of what caused the change. + * For example, it will fire when the local client reassigns a child, when part of a remote edit is applied to the + * node, or when the node has to be updated due to resolution of a merge conflict + * (for example a previously applied local change might be undone, then reapplied differently or not at all). + */ + nodeChanged(): void; +} + +export interface ObjectNodeChangeEvents { + /** + * Emitted by an object node after a batch of changes has been applied to the tree, if a change affected the node, + * where a change means that the value of one of its properties changed (i.e., the property's value is set + * to something else, including `undefined`). + * + * @remarks * This event is not emitted when: * * - Properties of a child node change. Notably, updates to an array node or a map node (like adding or removing @@ -95,24 +192,62 @@ export interface TreeChangeEvents { nodeChanged({ changedProperties, }: { readonly changedProperties: ReadonlySet }): void; +} +export interface MapNodeChangeEvents { /** - * Emitted by a node after a batch of changes has been applied to the tree, when something changed anywhere in the - * subtree rooted at it. + * Emitted by a map node after a batch of changes has been applied to the tree, if a change affected the node, where a + * change means that an entry was added, updated, or removed. * * @remarks - * This event is not emitted when the node itself is moved to a different location in the tree or removed from the tree. - * In that case it is emitted on the _parent_ node, not the node itself. + * This event is not emitted when: * - * The node itself is part of the subtree, so this event will be emitted even if the only changes are to the properties - * of the node itself. + * - Properties of a child node change. Notably, updates to an array node or a map node (like adding or removing + * elements/entries) will emit this event on the array/map node itself, but not on the node that contains the + * array/map node as one of its properties. + * + * - The node is moved to a different location in the tree or removed from the tree. + * In this case the event is emitted on the _parent_ node, not the node itself. + * + * For remote edits, this event is not guaranteed to occur in the same order or quantity that it did in + * the client that made the original edit. + * + * When it is emitted, the tree is guaranteed to be in-schema. + * + * @privateRemarks + * This event occurs whenever the apparent contents of the node instance change, regardless of what caused the change. + * For example, it will fire when the local client reassigns a child, when part of a remote edit is applied to the + * node, or when the node has to be updated due to resolution of a merge conflict + * (for example a previously applied local change might be undone, then reapplied differently or not at all). + */ + nodeChanged({ + changedProperties, + }: { readonly changedProperties: ReadonlySet }): void; +} + +export interface ArrayNodeChangeEvents { + /** + * Emitted by an array node after a batch of changes has been applied to the tree, if a change affected the node, + * where a change means that an element was added, removed, or moved. + * + * @remarks + * This event is not emitted when: + * + * - The node is moved to a different location in the tree or removed from the tree. + * In this case the event is emitted on the _parent_ node, not the node itself. * * For remote edits, this event is not guaranteed to occur in the same order or quantity that it did in * the client that made the original edit. * * When it is emitted, the tree is guaranteed to be in-schema. + * + * @privateRemarks + * This event occurs whenever the apparent contents of the node instance change, regardless of what caused the change. + * For example, it will fire when the local client reassigns a child, when part of a remote edit is applied to the + * node, or when the node has to be updated due to resolution of a merge conflict + * (for example a previously applied local change might be undone, then reapplied differently or not at all). */ - treeChanged(): void; + nodeChanged(): void; } /** @@ -174,6 +309,13 @@ export abstract class TreeNode implements WithType { */ public abstract get [typeNameSymbol](): string; + /** + * Adds a node kind symbol for stronger typing. + * @privateRemarks + * Subclasses provide more specific strings for this to get strong typing of otherwise type compatible nodes. + */ + public abstract get [nodeKindSymbol](): NodeKind; + /** * Provides `instanceof` support for testing if a value is a `TreeNode`. * @remarks diff --git a/packages/dds/tree/src/test/simple-tree/treeApi.spec.ts b/packages/dds/tree/src/test/simple-tree/treeApi.spec.ts index 92e1d373b95a..c296c1bded95 100644 --- a/packages/dds/tree/src/test/simple-tree/treeApi.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/treeApi.spec.ts @@ -19,7 +19,9 @@ import { type NodeFromSchema, SchemaFactory, treeNodeApi as Tree, + type TreeArrayNode, type TreeChangeEvents, + type TreeMapNode, TreeViewConfiguration, } from "../../simple-tree/index.js"; import { getView } from "../utils.js"; @@ -37,6 +39,7 @@ import { } from "../../simple-tree/leafNodeSchema.js"; // eslint-disable-next-line import/no-internal-modules import { tryGetSchema } from "../../simple-tree/treeNodeApi.js"; +import type { NodeChangedEventArgs } from "../../simple-tree/types.js"; const schema = new SchemaFactory("com.example"); @@ -782,8 +785,8 @@ describe("treeNodeApi", () => { const root = view.root; const eventLog: ReadonlySet[] = []; - Tree.on(root, "nodeChanged", ({ changedProperties }) => - eventLog.push(changedProperties), + Tree.on(root, "nodeChanged", (args: NodeChangedEventArgs) => + eventLog.push(args.changedProperties), ); const { forkView, forkCheckout } = getViewForForkedBranch(view); @@ -813,8 +816,8 @@ describe("treeNodeApi", () => { const root = view.root; const eventLog: ReadonlySet[] = []; - Tree.on(root, "nodeChanged", ({ changedProperties }) => - eventLog.push(changedProperties), + Tree.on(root, "nodeChanged", (args: NodeChangedEventArgs) => + eventLog.push(args.changedProperties), ); const { forkView, forkCheckout } = getViewForForkedBranch(view); @@ -830,7 +833,7 @@ describe("treeNodeApi", () => { assert.deepEqual(eventLog, [new Set(["key1", "key2", "key3"])]); }); - it(`'nodeChanged' does not include the names of changed properties (arrayNode)`, () => { + it(`'nodeChanged' does not have changed properties (arrayNode)`, () => { const sb = new SchemaFactory("test"); class TestNode extends sb.array("root", [sb.number]) {} @@ -838,10 +841,13 @@ describe("treeNodeApi", () => { view.initialize([1, 2]); const root = view.root; - const eventLog: ReadonlySet[] = []; - Tree.on(root, "nodeChanged", ({ changedProperties }) => - eventLog.push(changedProperties), - ); + let eventFireCount = 0; + // @ts-expect-error The Tree.on() overload for nodeChanged event for an array node should not have a payload. + // It still exists, but it should be an empty set for now. + Tree.on(root, "nodeChanged", (args: NodeChangedEventArgs) => { + eventFireCount++; + assert.equal(args.changedProperties, new Set()); + }); const { forkView, forkCheckout } = getViewForForkedBranch(view); @@ -853,10 +859,10 @@ describe("treeNodeApi", () => { view.checkout.merge(forkCheckout); - assert.deepEqual(eventLog, [new Set()]); + assert.equal(eventFireCount, 1); }); - it(`'nodeChanged' uses view keys, not stored keys, for the list of changed properties`, () => { + it(`'nodeChanged' on an object node uses view keys, not stored keys, for the list of changed properties`, () => { const sb = new SchemaFactory("test"); class TestNode extends sb.object("root", { prop1: sb.optional(sb.number, { key: "stored-prop1" }), @@ -867,8 +873,8 @@ describe("treeNodeApi", () => { const root = view.root; const eventLog: ReadonlySet[] = []; - Tree.on(root, "nodeChanged", ({ changedProperties }) => - eventLog.push(changedProperties), + Tree.on(root, "nodeChanged", (args: NodeChangedEventArgs) => + eventLog.push(args.changedProperties), ); const { forkView, forkCheckout } = getViewForForkedBranch(view);