Fix Tree.schema (#22185)

## Description

`Tree.schema` has changed from:

```typescript
schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
```
to:
```typescript
schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
```

See changeset for details.

## Breaking Changes

See changeset for details.

.changeset/loud-items-joke.md

@@ -0,0 +1,40 @@
+---
+"fluid-framework": minor
+"@fluidframework/tree": minor
+---
+---
+"section": "tree"
+---
+`Tree.schema` now returns `TreeNodeSchema`.
+
+The typing of `Tree.schema` has changed from:
+
+```typescript
+schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+```
+
+to:
+
+```typescript
+schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
+```
+
+The runtime behavior is unaffected: any code which worked and still compiles is fine and does not need changes.
+
+`Tree.schema` was changed to mitigate two different issues:
+
+1. It tried to give a more specific type based on the type of the passed in value.
+   When the type of the input is not known precisely (for example it is a union of node types like `Foo | Bar`, or `TreeNode` or even `TreeNode | TreeLeafValue`), this was fine since schema are covariant over their node type.
+   However when the input was more specific that the schema type, for example the type is simply `0`, this would result in unsound typing, since the create function could actually return values that did not conform with that schema (for example `schema.create(1)` for the number schema typed with `0` would return `1` with type `0`).
+2. The node type was provided to the incorrect type parameter of TreeNodeSchema.
+   The `TNode` parameter is the third one, not the fourth.
+   The fourth is `TBuild` which sets the input accepted to its create function or constructor.
+   Thus this code accidentally left `TNode` unset (which is good due to the above issue), but invalidly set `TBuild`.
+   `TBuild` is contravariant, so it has the opposite issue that setting `TNode` would have: if your input is simply typed as something general like `TreeNode`, then the returned schema would claim to be able to construct an instance given any `TreeNode`.
+   This is incorrect, and this typing has been removed.
+
+Fortunately it should be rare for code to be impacted by this issue.
+Any code which manually specified a generic type parameter to `Tree.schema()` will break, as well as code which assigned its result to an overly specifically typed variable.
+Code which used `typeof` on the returned schema could also break, though there are few use-cases for this so such code is not expected to exist.
+Currently it's very difficult to invoke the create function or constructor associated with a `TreeNodeSchema` as doing so already requires narrowing to `TreeNodeSchemaClass` or `TreeNodeSchemaNonClass`.
+It is possible some such code exists which will need to have an explicit cast added because it happened to work with the more specific (but incorrect) constructor input type.

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

@@ -514,7 +514,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

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

@@ -446,7 +446,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

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

@@ -446,7 +446,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

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

@@ -61,9 +61,7 @@ export interface TreeNodeApi {
 	/**
 	 * The schema information for this node.
 	 */
-	schema<T extends TreeNode | TreeLeafValue>(
-		node: T,
-	): TreeNodeSchema<string, NodeKind, unknown, T>;
+	schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
 
 	/**
 	 * Narrow the type of the given value if it satisfies the given schema.
@@ -196,9 +194,7 @@ export const treeNodeApi: TreeNodeApi = {
 			return (schema as TreeNodeSchema) === actualSchema;
 		}
 	},
-	schema<T extends TreeNode | TreeLeafValue>(
-		node: T,
-	): TreeNodeSchema<string, NodeKind, unknown, T> {
+	schema(node: TreeNode | TreeLeafValue): TreeNodeSchema {
 		return tryGetSchema(node) ?? fail("Not a tree node");
 	},
 	shortId(node: TreeNode): number | string | undefined {

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

@@ -5,6 +5,8 @@
 
 import { strict as assert } from "node:assert";
 
+import { MockHandle } from "@fluidframework/test-runtime-utils/internal";
+
 import {
 	CheckoutFlexTreeView,
 	type TransactionConstraint,
@@ -17,6 +19,8 @@ import {
 	type ValidateRecursiveSchema,
 	type TreeView,
 	type InsertableTypedNode,
+	type TreeNodeSchema,
+	type NodeFromSchema,
 } from "../../simple-tree/index.js";
 import { TestTreeProviderLite, createTestUndoRedoStacks, getView } from "../utils.js";
 
@@ -218,13 +222,14 @@ describe("treeApi", () => {
 			// TODO: Either enable when afterBatch is implemented, or delete if no longer relevant
 			it.skip("emits change events", () => {
 				const view = getTestObjectView();
-				let event = false;
-				view.events.on("rootChanged", () => (event = true));
+				let eventCount = 0;
+				view.events.on("rootChanged", () => (eventCount += 1));
 				view.root.content = 44;
+				assert.equal(eventCount, 1);
 				Tree.runTransaction(view, (root) => {
 					root.content = 43;
 				});
-				assert.equal(event, true);
+				assert.equal(eventCount, 2);
 			});
 
 			it.skip("emits change events on rollback", () => {
@@ -233,12 +238,35 @@ describe("treeApi", () => {
 				view.events.on("rootChanged", () => (eventCount += 1));
 				Tree.runTransaction(view, (r) => {
 					r.content = 43;
+					assert.equal(eventCount, 1);
 					return Tree.runTransaction.rollback;
 				});
 				assert.equal(eventCount, 2);
 			});
 		});
 
+		describe("schema", () => {
+			it("leaf", () => {
+				assert.equal(Tree.schema(null), schemaFactory.null);
+				assert.equal(Tree.schema(0), schemaFactory.number);
+				assert.equal(Tree.schema(false), schemaFactory.boolean);
+				assert.equal(Tree.schema(""), schemaFactory.string);
+				assert.equal(Tree.schema(new MockHandle(0)), schemaFactory.handle);
+
+				// Inferring the node type from the node in Tree.schema can incorrectly over-narrow.
+				// Ensure this does not happen:
+				const schema = Tree.schema(0);
+				type _check1 = requireAssignableTo<2, NodeFromSchema<typeof schema>>;
+			});
+
+			it("node", () => {
+				class Test extends schemaFactory.object("Test", {}) {}
+
+				const schema: TreeNodeSchema = Tree.schema(new Test({}));
+				assert.equal(schema, Test);
+			});
+		});
+
 		describe("invoked by passing a node", () => {
 			runCommonTransactionTests("view");
 

packages/framework/fluid-framework/api-report/fluid-framework.beta.api.md

@@ -818,7 +818,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

packages/framework/fluid-framework/api-report/fluid-framework.legacy.alpha.api.md

@@ -1215,7 +1215,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md

@@ -858,7 +858,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }

packages/framework/fluid-framework/api-report/fluid-framework.public.api.md

@@ -818,7 +818,7 @@ export interface TreeNodeApi {
     key(node: TreeNode): string | number;
     on<K extends keyof TreeChangeEvents>(node: TreeNode, eventName: K, listener: TreeChangeEvents[K]): () => void;
     parent(node: TreeNode): TreeNode | undefined;
-    schema<T extends TreeNode | TreeLeafValue>(node: T): TreeNodeSchema<string, NodeKind, unknown, T>;
+    schema(node: TreeNode | TreeLeafValue): TreeNodeSchema;
     shortId(node: TreeNode): number | string | undefined;
     status(node: TreeNode): TreeStatus;
 }