feat: Make NodeFromSchema work in more cases (microsoft#23089)

## Description NodeFromSchema now prefers the class based form if given a schema which implements a class based and non class based API. This shouldn't impact our public APIs (unless someone adds a static create function to their class based schema, in which case this is a fix), but internally we use TreeNodeSchemaBoth in some places, and if you subclass a type implementing TreeNodeSchemaBoth, before this change NodeFromSchema would give the less specialized type from the create function, ignoring the subclassing. This was confusing and has been fixed.
tylerbutler · Dec 3, 2024 · 2febdf8 · 2febdf8
1 parent 9e61721
commit 2febdf8
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 12 deletions.
diff --git a/packages/dds/tree/api-report/tree.alpha.api.md b/packages/dds/tree/api-report/tree.alpha.api.md
@@ -455,7 +455,7 @@ export interface NodeChangedData<TNode extends TreeNode = TreeNode> {
 }
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/dds/tree/api-report/tree.beta.api.md b/packages/dds/tree/api-report/tree.beta.api.md
@@ -245,7 +245,7 @@ export interface NodeChangedData<TNode extends TreeNode = TreeNode> {
 }
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/dds/tree/api-report/tree.legacy.alpha.api.md b/packages/dds/tree/api-report/tree.legacy.alpha.api.md
@@ -240,7 +240,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/dds/tree/api-report/tree.legacy.public.api.md b/packages/dds/tree/api-report/tree.legacy.public.api.md
@@ -240,7 +240,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/dds/tree/api-report/tree.public.api.md b/packages/dds/tree/api-report/tree.public.api.md
@@ -240,7 +240,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/dds/tree/src/simple-tree/schemaTypes.ts b/packages/dds/tree/src/simple-tree/schemaTypes.ts
@@ -24,6 +24,7 @@ import type {
 	TreeNodeSchemaClass,
 	TreeNode,
 	TreeNodeSchemaCore,
+	TreeNodeSchemaNonClass,
 } from "./core/index.js";
 import type { FieldKey } from "../core/index.js";
 import type { InsertableContent } from "./toMapTree.js";
@@ -759,15 +760,19 @@ export type InsertableTreeNodeFromAllowedTypes<TList extends AllowedTypes> =
 
 /**
  * Takes in `TreeNodeSchema[]` and returns a TypedNode union.
+ * @privateRemarks
+ * If a schema is both TreeNodeSchemaClass and TreeNodeSchemaNonClass, prefer TreeNodeSchemaClass since that includes subclasses properly.
  * @public
  */
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<
 	string,
 	NodeKind,
 	infer TNode
 >
 	? TNode
-	: never;
+	: T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode>
+		? TNode
+		: never;
 
 /**
  * Data which can be used as a node to be inserted.

diff --git a/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts b/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts
@@ -42,6 +42,8 @@ import type {
 	requireTrue,
 	UnionToIntersection,
 } from "../../util/index.js";
+// eslint-disable-next-line import/no-internal-modules
+import { objectSchema } from "../../simple-tree/objectNode.js";
 import { validateUsageError } from "../utils.js";
 
 const schema = new SchemaFactory("com.example");
@@ -216,6 +218,28 @@ describe("schemaTypes", () => {
 			type I13 = InsertableField<typeof booleanSchema>;
 			type _check13 = requireTrue<areSafelyAssignable<I13, boolean>>;
 		}
+
+		// NodeFromSchema
+		{
+			class Simple extends schema.object("A", { x: [schema.number] }) {}
+			class Customized extends schema.object("B", { x: [schema.number] }) {
+				public customized = true;
+			}
+
+			// Class that implements both TreeNodeSchemaNonClass and TreeNodeSchemaNonClass
+			class CustomizedBoth extends objectSchema("B", { x: [schema.number] }, true) {
+				public customized = true;
+			}
+
+			type TA = NodeFromSchema<typeof Simple>;
+			type _checkA = requireAssignableTo<TA, Simple>;
+
+			type TB = NodeFromSchema<typeof Customized>;
+			type _checkB = requireAssignableTo<TB, Customized>;
+
+			type TC = NodeFromSchema<typeof CustomizedBoth>;
+			type _checkC = requireAssignableTo<TC, CustomizedBoth>;
+		}
 	}
 
 	describe("insertable", () => {

diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md
@@ -811,7 +811,7 @@ export interface NodeChangedData<TNode extends TreeNode = TreeNode> {
 }
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.beta.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.beta.api.md
@@ -598,7 +598,7 @@ export interface NodeChangedData<TNode extends TreeNode = TreeNode> {
 }
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.legacy.alpha.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.legacy.alpha.api.md
@@ -896,7 +896,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md
@@ -629,7 +629,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;

diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.public.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.public.api.md
@@ -593,7 +593,7 @@ type NodeBuilderData<T extends TreeNodeSchemaCore<string, NodeKind, boolean>> =
 type NodeBuilderDataUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, unknown, infer TBuild> ? TBuild : never;
 
 // @public
-export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchema<string, NodeKind, infer TNode> ? TNode : never;
+export type NodeFromSchema<T extends TreeNodeSchema> = T extends TreeNodeSchemaClass<string, NodeKind, infer TNode> ? TNode : T extends TreeNodeSchemaNonClass<string, NodeKind, infer TNode> ? TNode : never;
 
 // @public
 type NodeFromSchemaUnsafe<T extends Unenforced<TreeNodeSchema>> = T extends TreeNodeSchemaUnsafe<string, NodeKind, infer TNode> ? TNode : never;