fix: tree: make field assignment safer

.changeset/metal-sloths-join.md

@@ -0,0 +1,24 @@
+---
+"fluid-framework": minor
+"@fluidframework/tree": minor
+---
+---
+"section": tree
+---
+
+Disallow some invalid and unsafe ObjectNode field assignments at compile time
+
+The compile time validation of the type of values assigned to ObjectNode fields is limited by TypeScript's limitations.
+Two cases which were actually possible to disallow and should be disallowed for consistency with runtime behavior and similar APIs were being allowed:
+
+1. [Identifier fields](https://fluidframework.com/docs/api/v2/fluid-framework/schemafactory-class#identifier-property):
+   Identifier fields are immutable, and setting them produces a runtime error.
+   This changes fixes them to no longer be typed as assignable.
+
+2. Fields with non-exact schema:
+   When non-exact scheme is used for a field (for example the schema is either a schema only allowing numbers or a schema only allowing strings) the field is no longer typed as assignable.
+   This matches how constructors and implicit node construction work.
+   For example when a node `Foo` has such an non-exact schema for field `bar`, you can no longer unsafely do `foo.bar = 5` just like how you could already not do `new Foo({bar: 5})`.
+
+This fix only applies to [`SchemaFactory.object`](https://fluidframework.com/docs/api/v2/fluid-framework/schemafactory-class#object-method).
+[`SchemaFactory.objectRecursive`](https://fluidframework.com/docs/api/v2/fluid-framework/schemafactory-class#objectrecursive-method) was unable to be updated to match due to TypeScript limitations on recursive types.

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

@@ -28,11 +28,19 @@ type ApplyKind<T, Kind extends FieldKind> = {
     [FieldKind.Identifier]: T;
 }[Kind];
 
+// @public
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+FieldKind.Required
+] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : never;
+
 // @public
 type ApplyKindInput<T, Kind extends FieldKind, DefaultsAreOptional extends boolean> = [
 Kind
 ] extends [FieldKind.Required] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : [Kind] extends [FieldKind.Identifier] ? DefaultsAreOptional extends true ? T | undefined : T : never;
 
+// @public
+export type AssignableTreeFieldFromImplicitField<TSchemaInput extends ImplicitFieldSchema, TSchema = UnionToIntersection<TSchemaInput>> = [TSchema] extends [FieldSchema<infer Kind, infer Types>] ? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind> : [TSchema] extends [ImplicitAllowedTypes] ? InsertableTreeNodeFromImplicitAllowedTypes<TSchema> : never;
+
 // @alpha
 export function asTreeViewAlpha<TSchema extends ImplicitFieldSchema>(view: TreeView<TSchema>): TreeViewAlpha<TSchema>;
 
@@ -470,7 +478,9 @@ export const noopValidator: JsonValidator;
 
 // @public
 type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-    -readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+    -readonly [Property in keyof T as Property extends string ? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never] ? never : Property : never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+    readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
 };
 
 // @public

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

@@ -17,11 +17,19 @@ type ApplyKind<T, Kind extends FieldKind> = {
     [FieldKind.Identifier]: T;
 }[Kind];
 
+// @public
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+FieldKind.Required
+] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : never;
+
 // @public
 type ApplyKindInput<T, Kind extends FieldKind, DefaultsAreOptional extends boolean> = [
 Kind
 ] extends [FieldKind.Required] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : [Kind] extends [FieldKind.Identifier] ? DefaultsAreOptional extends true ? T | undefined : T : never;
 
+// @public
+export type AssignableTreeFieldFromImplicitField<TSchemaInput extends ImplicitFieldSchema, TSchema = UnionToIntersection<TSchemaInput>> = [TSchema] extends [FieldSchema<infer Kind, infer Types>] ? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind> : [TSchema] extends [ImplicitAllowedTypes] ? InsertableTreeNodeFromImplicitAllowedTypes<TSchema> : never;
+
 // @public
 export enum CommitKind {
     Default = 0,
@@ -275,7 +283,9 @@ export enum NodeKind {
 
 // @public
 type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-    -readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+    -readonly [Property in keyof T as Property extends string ? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never] ? never : Property : never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+    readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
 };
 
 // @public

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

@@ -17,11 +17,19 @@ type ApplyKind<T, Kind extends FieldKind> = {
     [FieldKind.Identifier]: T;
 }[Kind];
 
+// @public
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+FieldKind.Required
+] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : never;
+
 // @public
 type ApplyKindInput<T, Kind extends FieldKind, DefaultsAreOptional extends boolean> = [
 Kind
 ] extends [FieldKind.Required] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : [Kind] extends [FieldKind.Identifier] ? DefaultsAreOptional extends true ? T | undefined : T : never;
 
+// @public
+export type AssignableTreeFieldFromImplicitField<TSchemaInput extends ImplicitFieldSchema, TSchema = UnionToIntersection<TSchemaInput>> = [TSchema] extends [FieldSchema<infer Kind, infer Types>] ? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind> : [TSchema] extends [ImplicitAllowedTypes] ? InsertableTreeNodeFromImplicitAllowedTypes<TSchema> : never;
+
 // @public
 export enum CommitKind {
     Default = 0,
@@ -270,7 +278,9 @@ export enum NodeKind {
 
 // @public
 type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-    -readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+    -readonly [Property in keyof T as Property extends string ? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never] ? never : Property : never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+    readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
 };
 
 // @public

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

@@ -17,11 +17,19 @@ type ApplyKind<T, Kind extends FieldKind> = {
     [FieldKind.Identifier]: T;
 }[Kind];
 
+// @public
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+FieldKind.Required
+] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : never;
+
 // @public
 type ApplyKindInput<T, Kind extends FieldKind, DefaultsAreOptional extends boolean> = [
 Kind
 ] extends [FieldKind.Required] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : [Kind] extends [FieldKind.Identifier] ? DefaultsAreOptional extends true ? T | undefined : T : never;
 
+// @public
+export type AssignableTreeFieldFromImplicitField<TSchemaInput extends ImplicitFieldSchema, TSchema = UnionToIntersection<TSchemaInput>> = [TSchema] extends [FieldSchema<infer Kind, infer Types>] ? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind> : [TSchema] extends [ImplicitAllowedTypes] ? InsertableTreeNodeFromImplicitAllowedTypes<TSchema> : never;
+
 // @public
 export enum CommitKind {
     Default = 0,
@@ -270,7 +278,9 @@ export enum NodeKind {
 
 // @public
 type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-    -readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+    -readonly [Property in keyof T as Property extends string ? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never] ? never : Property : never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+    readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
 };
 
 // @public

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

@@ -17,11 +17,19 @@ type ApplyKind<T, Kind extends FieldKind> = {
     [FieldKind.Identifier]: T;
 }[Kind];
 
+// @public
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+FieldKind.Required
+] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : never;
+
 // @public
 type ApplyKindInput<T, Kind extends FieldKind, DefaultsAreOptional extends boolean> = [
 Kind
 ] extends [FieldKind.Required] ? T : [Kind] extends [FieldKind.Optional] ? T | undefined : [Kind] extends [FieldKind.Identifier] ? DefaultsAreOptional extends true ? T | undefined : T : never;
 
+// @public
+export type AssignableTreeFieldFromImplicitField<TSchemaInput extends ImplicitFieldSchema, TSchema = UnionToIntersection<TSchemaInput>> = [TSchema] extends [FieldSchema<infer Kind, infer Types>] ? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind> : [TSchema] extends [ImplicitAllowedTypes] ? InsertableTreeNodeFromImplicitAllowedTypes<TSchema> : never;
+
 // @public
 export enum CommitKind {
     Default = 0,
@@ -270,7 +278,9 @@ export enum NodeKind {
 
 // @public
 type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-    -readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+    -readonly [Property in keyof T as Property extends string ? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never] ? never : Property : never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+    readonly [Property in keyof T]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
 };
 
 // @public

packages/dds/tree/src/index.ts

@@ -191,6 +191,8 @@ export {
 	type TreeBranch,
 	type TreeBranchEvents,
 	asTreeViewAlpha,
+	type AssignableTreeFieldFromImplicitField,
+	type ApplyKindAssignment,
 } from "./simple-tree/index.js";
 export {
 	SharedTree,

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

@@ -56,9 +56,12 @@ export type Unenforced<_DesiredExtendsConstraint> = unknown;
  */
 export type ObjectFromSchemaRecordUnsafe<
 	T extends Unenforced<RestrictiveStringRecord<ImplicitFieldSchema>>,
-> = {
-	-readonly [Property in keyof T]: TreeFieldFromImplicitFieldUnsafe<T[Property]>;
-};
+> =
+	// Due to https://github.com/microsoft/TypeScript/issues/43826 we can not set the desired setter type.
+	// Partial mitigation for this (used for non-recursive types) breaks compilation if used here, so recursive object end up allowing some unsafe assignments which will error at runtime.
+	{
+		-readonly [Property in keyof T]: TreeFieldFromImplicitFieldUnsafe<T[Property]>;
+	};
 
 /**
  * {@link Unenforced} version of {@link TreeNodeSchema}.

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

@@ -160,6 +160,8 @@ export {
 	type FieldHasDefault,
 	type InsertableObjectFromSchemaRecord,
 	type ObjectFromSchemaRecord,
+	type AssignableTreeFieldFromImplicitField,
+	type ApplyKindAssignment,
 	type TreeObjectNode,
 	setField,
 } from "./objectNode.js";

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

@@ -26,6 +26,7 @@ import {
 	normalizeFieldSchema,
 	type ImplicitAllowedTypes,
 	FieldKind,
+	type InsertableTreeNodeFromImplicitAllowedTypes,
 } from "./schemaTypes.js";
 import {
 	type TreeNodeSchema,
@@ -41,7 +42,12 @@ import {
 	getOrCreateInnerNode,
 } from "./core/index.js";
 import { mapTreeFromNodeData, type InsertableContent } from "./toMapTree.js";
-import { type RestrictiveStringRecord, fail, type FlattenKeys } from "../util/index.js";
+import {
+	type RestrictiveStringRecord,
+	fail,
+	type FlattenKeys,
+	type UnionToIntersection,
+} from "../util/index.js";
 import type { ObjectNodeSchema, ObjectNodeSchemaInternalData } from "./objectNodeTypes.js";
 import { TreeNodeValid, type MostDerivedData } from "./treeNodeValid.js";
 import { getUnhydratedContext } from "./createContext.js";
@@ -51,11 +57,52 @@ import { getUnhydratedContext } from "./createContext.js";
  * @system @public
  */
 export type ObjectFromSchemaRecord<T extends RestrictiveStringRecord<ImplicitFieldSchema>> = {
-	-readonly [Property in keyof T]: Property extends string
+	// Due to https://github.com/microsoft/TypeScript/issues/43826 we can not set the desired setter type,
+	// but we can at least remove the setter (by setting the key to never) when there should be no setter.
+	-readonly [Property in keyof T as Property extends string
+		? [AssignableTreeFieldFromImplicitField<T[Property]>] extends [never]
+			? never
+			: Property
+		: never]: Property extends string ? TreeFieldFromImplicitField<T[Property]> : unknown;
+} & {
+	readonly [Property in keyof T]: Property extends string
 		? TreeFieldFromImplicitField<T[Property]>
 		: unknown;
 };
 
+/**
+ * Type of content that can be assigned to a field of the given schema.
+ *
+ * @see {@link Input}
+ *
+ * @typeparam TSchemaInput - Schema to process.
+ * @typeparam TSchema - Do not specify: default value used as implementation detail.
+ * @system @public
+ */
+export type AssignableTreeFieldFromImplicitField<
+	TSchemaInput extends ImplicitFieldSchema,
+	TSchema = UnionToIntersection<TSchemaInput>,
+> = [TSchema] extends [FieldSchema<infer Kind, infer Types>]
+	? ApplyKindAssignment<InsertableTreeNodeFromImplicitAllowedTypes<Types>, Kind>
+	: [TSchema] extends [ImplicitAllowedTypes]
+		? InsertableTreeNodeFromImplicitAllowedTypes<TSchema>
+		: never;
+
+/**
+ * Suitable for assignment.
+ *
+ * @see {@link Input}
+ * @system @public
+ */
+export type ApplyKindAssignment<T, Kind extends FieldKind> = [Kind] extends [
+	FieldKind.Required,
+]
+	? T
+	: [Kind] extends [FieldKind.Optional]
+		? T | undefined
+		: // Unknown, non-exact and identifier fields are not assignable.
+			never;
+
 /**
  * A {@link TreeNode} which modules a JavaScript object.
  * @remarks

packages/dds/tree/src/test/shared-tree-core/sharedTreeCore.spec.ts

@@ -294,9 +294,9 @@ describe("SharedTreeCore", () => {
 		});
 
 		const sf = new SchemaFactory("0x4a6 repro");
-		const TestNode = sf.objectRecursive("test node", {
+		class TestNode extends sf.objectRecursive("test node", {
 			child: sf.optionalRecursive([() => TestNode, sf.number]),
-		});
+		}) {}
 
 		const tree2 = await factory.load(
 			dataStoreRuntime2,