From 2febdf8f86daa37e26a8f4737d4c7adaee0d7da6 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:10:48 -0800 Subject: [PATCH] feat: Make NodeFromSchema work in more cases (#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. --- .../dds/tree/api-report/tree.alpha.api.md | 2 +- packages/dds/tree/api-report/tree.beta.api.md | 2 +- .../tree/api-report/tree.legacy.alpha.api.md | 2 +- .../tree/api-report/tree.legacy.public.api.md | 2 +- .../dds/tree/api-report/tree.public.api.md | 2 +- .../dds/tree/src/simple-tree/schemaTypes.ts | 9 +++++-- .../src/test/simple-tree/schemaTypes.spec.ts | 24 +++++++++++++++++++ .../api-report/fluid-framework.alpha.api.md | 2 +- .../api-report/fluid-framework.beta.api.md | 2 +- .../fluid-framework.legacy.alpha.api.md | 2 +- .../fluid-framework.legacy.public.api.md | 2 +- .../api-report/fluid-framework.public.api.md | 2 +- 12 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/dds/tree/api-report/tree.alpha.api.md b/packages/dds/tree/api-report/tree.alpha.api.md index 9734f0acb8d6..128b3b3bb851 100644 --- 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 { } // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? TNode : never; diff --git a/packages/dds/tree/api-report/tree.beta.api.md b/packages/dds/tree/api-report/tree.beta.api.md index d2b12ce272ef..34f2e4f70fd0 100644 --- 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 { } // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index 184c22baab00..c4a4c68bb46a 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index 1306818b234b..c22b34b8452f 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? TNode : never; diff --git a/packages/dds/tree/api-report/tree.public.api.md b/packages/dds/tree/api-report/tree.public.api.md index 1306818b234b..c22b34b8452f 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? TNode : never; diff --git a/packages/dds/tree/src/simple-tree/schemaTypes.ts b/packages/dds/tree/src/simple-tree/schemaTypes.ts index 83979f815c25..fd64bab4ba35 100644 --- 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 = /** * 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< +export type NodeFromSchema = T extends TreeNodeSchemaClass< string, NodeKind, infer TNode > ? TNode - : never; + : T extends TreeNodeSchemaNonClass + ? 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 index 23e9b05d4799..eff7319dd90a 100644 --- 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; type _check13 = requireTrue>; } + + // 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; + type _checkA = requireAssignableTo; + + type TB = NodeFromSchema; + type _checkB = requireAssignableTo; + + type TC = NodeFromSchema; + type _checkC = requireAssignableTo; + } } 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 index cbcc965c51ce..752b27794015 100644 --- 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 { } // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index a5f825fc72c3..97d42ecbefe5 100644 --- 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 { } // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index d350b24c62ca..b19a296b2dd9 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index f498e162e94b..b6be5f6f1a25 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? 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 index f0e4c2544a35..39e0f34b993e 100644 --- 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> = type NodeBuilderDataUnsafe> = T extends TreeNodeSchemaUnsafe ? TBuild : never; // @public -export type NodeFromSchema = T extends TreeNodeSchema ? TNode : never; +export type NodeFromSchema = T extends TreeNodeSchemaClass ? TNode : T extends TreeNodeSchemaNonClass ? TNode : never; // @public type NodeFromSchemaUnsafe> = T extends TreeNodeSchemaUnsafe ? TNode : never;