diff --git a/.changeset/gold-badgers-float.md b/.changeset/gold-badgers-float.md new file mode 100644 index 000000000000..c7062f38abd7 --- /dev/null +++ b/.changeset/gold-badgers-float.md @@ -0,0 +1,23 @@ +--- +"@fluidframework/shared-object-base": minor +--- + +Added typeguard for SharedObjectKinds + +In the 2.0 release of Fluid, the concrete class implementations for DDSes were hidden from Fluid's API surface. +This made `instanceof` checks fail to work correctly. +There were ways to work around this in application code, but they involved boilerplate which required more understanding of Fluid internals than should be necessary. +This change adds a drop-in replacement to `instanceof`: the `.is()` method to `SharedObjectKind`. +For example: + +```typescript +// Works in FluidFramework 1.0 but not in the initial release of FluidFramework 2.0: +if (myObject instanceof SharedString) { + // do something +} + +// With this change, that code can now be written like so: +if (SharedString.is(myObject)) { + // do something +} +``` diff --git a/packages/dds/cell/package.json b/packages/dds/cell/package.json index 3443ffa43e96..8d95d7dcca61 100644 --- a/packages/dds/cell/package.json +++ b/packages/dds/cell/package.json @@ -135,6 +135,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_SharedCell": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/cell/src/test/types/validateCellPrevious.generated.ts b/packages/dds/cell/src/test/types/validateCellPrevious.generated.ts index fc7a38f967e3..dcc03f06674d 100644 --- a/packages/dds/cell/src/test/types/validateCellPrevious.generated.ts +++ b/packages/dds/cell/src/test/types/validateCellPrevious.generated.ts @@ -103,6 +103,7 @@ declare type current_as_old_for_InterfaceDeclaration_ISharedCellEvents = require * typeValidation.broken: * "VariableDeclaration_SharedCell": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedCell = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/counter/package.json b/packages/dds/counter/package.json index c425071cb21b..4669cc482b45 100644 --- a/packages/dds/counter/package.json +++ b/packages/dds/counter/package.json @@ -152,6 +152,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_SharedCounter": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/counter/src/test/types/validateCounterPrevious.generated.ts b/packages/dds/counter/src/test/types/validateCounterPrevious.generated.ts index 398c54452e50..92b09dd20e9d 100644 --- a/packages/dds/counter/src/test/types/validateCounterPrevious.generated.ts +++ b/packages/dds/counter/src/test/types/validateCounterPrevious.generated.ts @@ -58,6 +58,7 @@ declare type current_as_old_for_InterfaceDeclaration_ISharedCounterEvents = requ * typeValidation.broken: * "VariableDeclaration_SharedCounter": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedCounter = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/map/package.json b/packages/dds/map/package.json index 905a50d5d2a0..2947bef25087 100644 --- a/packages/dds/map/package.json +++ b/packages/dds/map/package.json @@ -166,6 +166,13 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_SharedMap": { + "forwardCompat": false + }, + "VariableDeclaration_SharedDirectory": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/map/src/test/types/validateMapPrevious.generated.ts b/packages/dds/map/src/test/types/validateMapPrevious.generated.ts index ae5a6a60b3b8..eea3d1e4b39d 100644 --- a/packages/dds/map/src/test/types/validateMapPrevious.generated.ts +++ b/packages/dds/map/src/test/types/validateMapPrevious.generated.ts @@ -229,6 +229,7 @@ declare type current_as_old_for_ClassDeclaration_MapFactory = requireAssignableT * typeValidation.broken: * "VariableDeclaration_SharedDirectory": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedDirectory = requireAssignableTo, TypeOnly> /* @@ -265,6 +266,7 @@ declare type current_as_old_for_TypeAliasDeclaration_SharedDirectory = requireAs * typeValidation.broken: * "VariableDeclaration_SharedMap": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedMap = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/matrix/package.json b/packages/dds/matrix/package.json index 733f9994b304..f416f2d98a58 100644 --- a/packages/dds/matrix/package.json +++ b/packages/dds/matrix/package.json @@ -169,6 +169,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_SharedMatrix": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/matrix/src/test/types/validateMatrixPrevious.generated.ts b/packages/dds/matrix/src/test/types/validateMatrixPrevious.generated.ts index e2ba5a2c323a..bbb9971b61eb 100644 --- a/packages/dds/matrix/src/test/types/validateMatrixPrevious.generated.ts +++ b/packages/dds/matrix/src/test/types/validateMatrixPrevious.generated.ts @@ -112,6 +112,7 @@ declare type current_as_old_for_TypeAliasDeclaration_MatrixItem = requireAssigna * typeValidation.broken: * "VariableDeclaration_SharedMatrix": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedMatrix = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/ordered-collection/package.json b/packages/dds/ordered-collection/package.json index ae86cad24d06..d3e99b2d4957 100644 --- a/packages/dds/ordered-collection/package.json +++ b/packages/dds/ordered-collection/package.json @@ -156,6 +156,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_ConsensusQueue": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/ordered-collection/src/test/types/validateOrderedCollectionPrevious.generated.ts b/packages/dds/ordered-collection/src/test/types/validateOrderedCollectionPrevious.generated.ts index 6cfecda2a2cf..4ffb57b5340b 100644 --- a/packages/dds/ordered-collection/src/test/types/validateOrderedCollectionPrevious.generated.ts +++ b/packages/dds/ordered-collection/src/test/types/validateOrderedCollectionPrevious.generated.ts @@ -58,6 +58,7 @@ declare type current_as_old_for_ClassDeclaration_ConsensusOrderedCollection = re * typeValidation.broken: * "VariableDeclaration_ConsensusQueue": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_ConsensusQueue = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/register-collection/package.json b/packages/dds/register-collection/package.json index 03c0af04be1c..28cd629f23a6 100644 --- a/packages/dds/register-collection/package.json +++ b/packages/dds/register-collection/package.json @@ -153,6 +153,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_ConsensusRegisterCollection": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/register-collection/src/test/types/validateRegisterCollectionPrevious.generated.ts b/packages/dds/register-collection/src/test/types/validateRegisterCollectionPrevious.generated.ts index 5ed772e922c9..6e033968af59 100644 --- a/packages/dds/register-collection/src/test/types/validateRegisterCollectionPrevious.generated.ts +++ b/packages/dds/register-collection/src/test/types/validateRegisterCollectionPrevious.generated.ts @@ -22,6 +22,7 @@ declare type MakeUnusedImportErrorsGoAway = TypeOnly | MinimalType | Fu * typeValidation.broken: * "VariableDeclaration_ConsensusRegisterCollection": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_ConsensusRegisterCollection = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/sequence/package.json b/packages/dds/sequence/package.json index 50f8ccfa4457..78f0844bb12b 100644 --- a/packages/dds/sequence/package.json +++ b/packages/dds/sequence/package.json @@ -194,6 +194,9 @@ "broken": { "InterfaceDeclaration_SequenceOptions": { "backCompat": false + }, + "VariableDeclaration_SharedString": { + "forwardCompat": false } } } diff --git a/packages/dds/sequence/src/test/types/validateSequencePrevious.generated.ts b/packages/dds/sequence/src/test/types/validateSequencePrevious.generated.ts index 2ee8de616c36..6539b67235bc 100644 --- a/packages/dds/sequence/src/test/types/validateSequencePrevious.generated.ts +++ b/packages/dds/sequence/src/test/types/validateSequencePrevious.generated.ts @@ -923,6 +923,7 @@ declare type current_as_old_for_ClassDeclaration_SharedSequence = requireAssigna * typeValidation.broken: * "VariableDeclaration_SharedString": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedString = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/shared-object-base/api-report/shared-object-base.beta.api.md b/packages/dds/shared-object-base/api-report/shared-object-base.beta.api.md index 3f963b2815a1..a31e19ceffaa 100644 --- a/packages/dds/shared-object-base/api-report/shared-object-base.beta.api.md +++ b/packages/dds/shared-object-base/api-report/shared-object-base.beta.api.md @@ -6,6 +6,7 @@ // @public @sealed export interface SharedObjectKind extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // (No @packageDocumentation comment for this package) diff --git a/packages/dds/shared-object-base/api-report/shared-object-base.legacy.alpha.api.md b/packages/dds/shared-object-base/api-report/shared-object-base.legacy.alpha.api.md index 661ae4569621..7adbb4b97953 100644 --- a/packages/dds/shared-object-base/api-report/shared-object-base.legacy.alpha.api.md +++ b/packages/dds/shared-object-base/api-report/shared-object-base.legacy.alpha.api.md @@ -91,6 +91,7 @@ export abstract class SharedObjectCore extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // (No @packageDocumentation comment for this package) diff --git a/packages/dds/shared-object-base/api-report/shared-object-base.legacy.public.api.md b/packages/dds/shared-object-base/api-report/shared-object-base.legacy.public.api.md index c281d8e0f61b..d0858f014520 100644 --- a/packages/dds/shared-object-base/api-report/shared-object-base.legacy.public.api.md +++ b/packages/dds/shared-object-base/api-report/shared-object-base.legacy.public.api.md @@ -6,6 +6,7 @@ // @public @sealed export interface SharedObjectKind extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // (No @packageDocumentation comment for this package) diff --git a/packages/dds/shared-object-base/api-report/shared-object-base.public.api.md b/packages/dds/shared-object-base/api-report/shared-object-base.public.api.md index c281d8e0f61b..d0858f014520 100644 --- a/packages/dds/shared-object-base/api-report/shared-object-base.public.api.md +++ b/packages/dds/shared-object-base/api-report/shared-object-base.public.api.md @@ -6,6 +6,7 @@ // @public @sealed export interface SharedObjectKind extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // (No @packageDocumentation comment for this package) diff --git a/packages/dds/shared-object-base/src/sharedObject.ts b/packages/dds/shared-object-base/src/sharedObject.ts index f6e80b77897c..0d7d12c93344 100644 --- a/packages/dds/shared-object-base/src/sharedObject.ts +++ b/packages/dds/shared-object-base/src/sharedObject.ts @@ -7,13 +7,15 @@ import { EventEmitterEventType } from "@fluid-internal/client-utils"; import { AttachState } from "@fluidframework/container-definitions"; import type { IDeltaManager } from "@fluidframework/container-definitions/internal"; import { ITelemetryBaseProperties, type ErasedType } from "@fluidframework/core-interfaces"; -import { type IFluidHandleInternal } from "@fluidframework/core-interfaces/internal"; +import { + type IFluidHandleInternal, + type IFluidLoadable, +} from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import { IChannelServices, IChannelStorageService, -} from "@fluidframework/datastore-definitions/internal"; -import { + type IChannel, IChannelAttributes, type IChannelFactory, IFluidDataStoreRuntime, @@ -868,9 +870,14 @@ export interface ISharedObjectKind { * @sealed * @public */ -// eslint-disable-next-line @typescript-eslint/no-empty-interface export interface SharedObjectKind - extends ErasedType {} + extends ErasedType { + /** + * Check whether an {@link @fluidframework/core-interfaces#IFluidLoadable} is an instance of this shared object kind. + * @remarks This should be used in place of `instanceof` checks for shared objects, as their actual classes are not exported in Fluid's public API. + */ + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; +} /** * Utility for creating ISharedObjectKind instances. @@ -884,7 +891,8 @@ export interface SharedObjectKind export function createSharedObjectKind( factory: (new () => IChannelFactory) & { readonly Type: string }, ): ISharedObjectKind & SharedObjectKind { - const result: ISharedObjectKind = { + const result: ISharedObjectKind & + Omit, "brand"> = { getFactory(): IChannelFactory { return new factory(); }, @@ -892,7 +900,16 @@ export function createSharedObjectKind( create(runtime: IFluidDataStoreRuntime, id?: string): TSharedObject { return runtime.createChannel(id, factory.Type) as TSharedObject; }, + + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject { + return isChannel(value) && value.attributes.type === factory.Type; + }, }; return result as typeof result & SharedObjectKind; } + +function isChannel(loadable: IFluidLoadable): loadable is IChannel { + // This assumes no other IFluidLoadable has an `attributes` field, and thus may not be fully robust. + return (loadable as IChannel).attributes !== undefined; +} diff --git a/packages/dds/shared-object-base/src/test/createSharedObjectKind.spec.ts b/packages/dds/shared-object-base/src/test/createSharedObjectKind.spec.ts new file mode 100644 index 000000000000..f8de39c128c7 --- /dev/null +++ b/packages/dds/shared-object-base/src/test/createSharedObjectKind.spec.ts @@ -0,0 +1,84 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; + +import type { IFluidLoadable } from "@fluidframework/core-interfaces"; +import type { + IChannel, + IChannelAttributes, + IChannelFactory, + IChannelServices, + IFluidDataStoreRuntime, +} from "@fluidframework/datastore-definitions/internal"; +import { MockFluidDataStoreRuntime } from "@fluidframework/test-runtime-utils/internal"; + +import { createSharedObjectKind } from "../sharedObject.js"; + +interface IFoo { + foo: string; +} +class SharedFooFactory implements IChannelFactory { + public static readonly Type: string = "SharedFoo"; + public readonly type: string = SharedFooFactory.Type; + public readonly attributes: IChannelAttributes = { + type: SharedFooFactory.Type, + snapshotFormatVersion: "0.1", + }; + async load( + runtime: IFluidDataStoreRuntime, + id: string, + services: IChannelServices, + channelAttributes: Readonly, + ): Promise { + throw new Error("Method not implemented."); + } + create(runtime: IFluidDataStoreRuntime, id: string): IFoo & IChannel { + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + return { + foo: "bar", + attributes: this.attributes, + id, + // Note: other IChannel methods aren't relevant + } as IFoo & IChannel; + } +} + +const SharedFoo = createSharedObjectKind(SharedFooFactory); + +describe("createSharedObjectKind's return type", () => { + it("delegates to runtime.createChannel on creation", () => { + const createChannelCalls: [id: string | undefined, type: string][] = []; + const runtime = new MockFluidDataStoreRuntime(); + runtime.createChannel = (id: string | undefined, type: string) => { + createChannelCalls.push([id, type]); + return null as unknown as IChannel; + }; + SharedFoo.create(runtime); + assert.deepEqual(createChannelCalls, [[undefined, SharedFooFactory.Type]]); + createChannelCalls.length = 0; + SharedFoo.create(runtime, "test-id"); + assert.deepEqual(createChannelCalls, [["test-id", SharedFooFactory.Type]]); + }); + + describe(".is", () => { + it("returns true for objects created by the factory", () => { + const factory = SharedFoo.getFactory(); + const foo = factory.create(new MockFluidDataStoreRuntime(), "test-id"); + assert(SharedFoo.is(foo)); + }); + describe("returns false for", () => { + const cases: [name: string, obj: unknown][] = [ + ["object without attributres", {}], + ["object with wrong type", { attributes: { type: "NotSharedFoo" } }], + ]; + for (const [name, obj] of cases) { + it(name, () => { + assert(!SharedFoo.is(obj as IFluidLoadable)); + }); + } + }); + }); +}); diff --git a/packages/dds/shared-summary-block/package.json b/packages/dds/shared-summary-block/package.json index cf266222fa69..d96b34620849 100644 --- a/packages/dds/shared-summary-block/package.json +++ b/packages/dds/shared-summary-block/package.json @@ -154,6 +154,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_SharedSummaryBlock": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/shared-summary-block/src/test/types/validateSharedSummaryBlockPrevious.generated.ts b/packages/dds/shared-summary-block/src/test/types/validateSharedSummaryBlockPrevious.generated.ts index 2885221c59e6..0220b2bfa3ae 100644 --- a/packages/dds/shared-summary-block/src/test/types/validateSharedSummaryBlockPrevious.generated.ts +++ b/packages/dds/shared-summary-block/src/test/types/validateSharedSummaryBlockPrevious.generated.ts @@ -40,6 +40,7 @@ declare type current_as_old_for_InterfaceDeclaration_ISharedSummaryBlock = requi * typeValidation.broken: * "VariableDeclaration_SharedSummaryBlock": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_SharedSummaryBlock = requireAssignableTo, TypeOnly> /* diff --git a/packages/dds/task-manager/package.json b/packages/dds/task-manager/package.json index f924af7e6014..c4befc608baa 100644 --- a/packages/dds/task-manager/package.json +++ b/packages/dds/task-manager/package.json @@ -157,6 +157,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "VariableDeclaration_TaskManager": { + "forwardCompat": false + } + } } } diff --git a/packages/dds/task-manager/src/test/types/validateTaskManagerPrevious.generated.ts b/packages/dds/task-manager/src/test/types/validateTaskManagerPrevious.generated.ts index 4085955f47fd..25076a46172a 100644 --- a/packages/dds/task-manager/src/test/types/validateTaskManagerPrevious.generated.ts +++ b/packages/dds/task-manager/src/test/types/validateTaskManagerPrevious.generated.ts @@ -76,6 +76,7 @@ declare type current_as_old_for_TypeAliasDeclaration_TaskEventListener = require * typeValidation.broken: * "VariableDeclaration_TaskManager": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_VariableDeclaration_TaskManager = requireAssignableTo, TypeOnly> /* 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 eda8e09f358f..c6aa898611d4 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 @@ -682,6 +682,7 @@ type ScopedSchemaName extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // @public 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 2c2b26ebeb60..0369fcd5b8c4 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 @@ -1075,6 +1075,7 @@ export type SharedMap = ISharedMap; // @public @sealed export interface SharedObjectKind extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // @alpha 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 ca90fc18b187..d30243d84a6d 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 @@ -682,6 +682,7 @@ type ScopedSchemaName extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // @public 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 ca90fc18b187..d30243d84a6d 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 @@ -682,6 +682,7 @@ type ScopedSchemaName extends ErasedType { + is(value: IFluidLoadable): value is IFluidLoadable & TSharedObject; } // @public diff --git a/packages/framework/fluid-static/package.json b/packages/framework/fluid-static/package.json index 4d71e8a5f5bc..1e5fa0f33091 100644 --- a/packages/framework/fluid-static/package.json +++ b/packages/framework/fluid-static/package.json @@ -139,6 +139,10 @@ "typescript": "~5.4.5" }, "typeValidation": { - "broken": {} + "broken": { + "InterfaceDeclaration_ContainerSchema": { + "forwardCompat": false + } + } } } diff --git a/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.generated.ts b/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.generated.ts index c225eb3e208e..bbbc5f7d4b49 100644 --- a/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.generated.ts +++ b/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.generated.ts @@ -58,6 +58,7 @@ declare type current_as_old_for_TypeAliasDeclaration_ContainerAttachProps = requ * typeValidation.broken: * "InterfaceDeclaration_ContainerSchema": {"forwardCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type old_as_current_for_InterfaceDeclaration_ContainerSchema = requireAssignableTo, TypeOnly> /*