From fd7394c6adc228d5d576f8c605afa2293414bfd3 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Thu, 17 Apr 2025 19:29:34 -0700 Subject: [PATCH 1/6] SharedObjectFromKernel --- packages/dds/shared-object-base/package.json | 1 + packages/dds/shared-object-base/src/index.ts | 10 + .../shared-object-base/src/sharedObject.ts | 3 + .../src/sharedObjectKernel.ts | 473 ++++++++++++++++++ .../dds/tree/src/shared-tree/sharedTree.ts | 28 +- .../src/test/shared-tree/sharedTree.spec.ts | 22 +- packages/dds/tree/src/treeFactory.ts | 228 ++------- pnpm-lock.yaml | 21 +- 8 files changed, 593 insertions(+), 193 deletions(-) create mode 100644 packages/dds/shared-object-base/src/sharedObjectKernel.ts diff --git a/packages/dds/shared-object-base/package.json b/packages/dds/shared-object-base/package.json index f91d167239c9..860713ba6dd4 100644 --- a/packages/dds/shared-object-base/package.json +++ b/packages/dds/shared-object-base/package.json @@ -123,6 +123,7 @@ "@fluidframework/datastore": "workspace:~", "@fluidframework/datastore-definitions": "workspace:~", "@fluidframework/driver-definitions": "workspace:~", + "@fluidframework/id-compressor": "workspace:~", "@fluidframework/runtime-definitions": "workspace:~", "@fluidframework/runtime-utils": "workspace:~", "@fluidframework/telemetry-utils": "workspace:~", diff --git a/packages/dds/shared-object-base/src/index.ts b/packages/dds/shared-object-base/src/index.ts index 8b81bed1afdd..d8f8e2c9c30f 100644 --- a/packages/dds/shared-object-base/src/index.ts +++ b/packages/dds/shared-object-base/src/index.ts @@ -21,3 +21,13 @@ export { type IChannelView, } from "./utils.js"; export { ValueType } from "./valueType.js"; +export { + SharedKernel, + thisWrap, + KernelArgs, + makeSharedObjectKind, + SharedKernelFactory, + FactoryOut, + SharedObjectOptions, + mergeAPIs, +} from "./sharedObjectKernel.js"; diff --git a/packages/dds/shared-object-base/src/sharedObject.ts b/packages/dds/shared-object-base/src/sharedObject.ts index 07f1c7b64ecc..34515070bd74 100644 --- a/packages/dds/shared-object-base/src/sharedObject.ts +++ b/packages/dds/shared-object-base/src/sharedObject.ts @@ -765,6 +765,9 @@ export abstract class SharedObject< id: string, runtime: IFluidDataStoreRuntime, attributes: IChannelAttributes, + /** + * The prefix to use for telemetry events emitted by this object. + */ private readonly telemetryContextPrefix: string, ) { super(id, runtime, attributes); diff --git a/packages/dds/shared-object-base/src/sharedObjectKernel.ts b/packages/dds/shared-object-base/src/sharedObjectKernel.ts new file mode 100644 index 000000000000..28534bd0f0c2 --- /dev/null +++ b/packages/dds/shared-object-base/src/sharedObjectKernel.ts @@ -0,0 +1,473 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import type { TypedEventEmitter } from "@fluid-internal/client-utils"; +import type { IFluidLoadable } from "@fluidframework/core-interfaces"; +import { assert, fail } from "@fluidframework/core-utils/internal"; +import { + IChannelStorageService, + type IChannel, + type IChannelAttributes, + type IChannelFactory, + type IChannelServices, + type IFluidDataStoreRuntime, +} from "@fluidframework/datastore-definitions/internal"; +import type { IIdCompressor } from "@fluidframework/id-compressor/internal"; +import { + ISummaryTreeWithStats, + ITelemetryContext, + type IExperimentalIncrementalSummaryContext, + type IRuntimeMessageCollection, +} from "@fluidframework/runtime-definitions/internal"; +import type { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal"; + +import { IFluidSerializer } from "./serializer.js"; +import { + createSharedObjectKind, + SharedObject, + type ISharedObjectKind, + type SharedObjectKind, +} from "./sharedObject.js"; +import { ISharedObjectEvents, type ISharedObject } from "./types.js"; +import type { IChannelView } from "./utils.js"; + +/** + * Functionality specific to a particular kind of shared object. + * @remarks + * SharedObjects expose APIs for two consumers: + * + * 1. The runtime, which uses the SharedObject to summarize, load and apply ops. + * 2. The app, who uses the SharedObject to read and write data. + * + * There is some common functionality all shared objects use, provided by {@link SharedObject}. + * SharedKernel describes the portion of the behavior required by the runtime which + * differs between different kinds of shared objects. + * + * {@link makeSharedObjectKind} is then used to wrap up the kernel into a full {@link ISharedObject} implementation. + * The runtime specific APIs are then type erased into a {@link SharedObjectKind}. + * @privateRemarks + * Unlike the `SharedObject` class, this interface is internal, and thus can be adjusted more easily. + * Therefore this interface is not intended to address all needs, and will likely need small changes as it gets more adoption. + * + * @internal + */ +export interface SharedKernel { + /** + * {@inheritDoc SharedObject.summarizeCore} + */ + summarizeCore( + serializer: IFluidSerializer, + telemetryContext: ITelemetryContext | undefined, + incrementalSummaryContext: IExperimentalIncrementalSummaryContext | undefined, + ): ISummaryTreeWithStats; + + /** + * {@inheritDoc SharedObjectCore.onDisconnect} + */ + onDisconnect(): void; + + /** + * {@inheritDoc SharedObjectCore.reSubmitCore} + */ + reSubmitCore(content: unknown, localOpMetadata: unknown): void; + + /** + * {@inheritDoc SharedObjectCore.applyStashedOp} + */ + applyStashedOp(content: unknown): void; + + /** + * {@inheritDoc SharedObjectCore.processMessagesCore} + */ + processMessagesCore(messagesCollection: IRuntimeMessageCollection): void; + + /** + * {@inheritDoc SharedObjectCore.rollback} + */ + rollback?(content: unknown, localOpMetadata: unknown): void; + + /** + * {@inheritDoc SharedObjectCore.didAttach} + */ + didAttach?(): void; +} + +/** + * SharedObject implementation that delegates to a SharedKernel. + * @typeParam TOut - The type of the object exposed to the app. + * Once initialized instances of this class forward properties to the `TOut` val;ue provided by the factory. + * See {@link mergeAPIs} for more limitations. + * + * @remarks + * The App facing API (TOut) needs to be implemented by this object which also has to implement the runtime facing API (ISharedObject). + * + * Requiring both of these to be implemented by the same object adds some otherwise unnecessary coupling. + * This class is a workaround for that, which takes separate implementations of the two APIs and merges them into one using {@link mergeAPIs}. + */ +class SharedObjectFromKernel< + TOut extends object, + TEvent extends ISharedObjectEvents, +> extends SharedObject { + /** + * Lazy init here so kernel can be constructed in loadCore when loading from existing data. + * + * Explicit initialization to undefined is done so Proxy knows this property is from this class (via `Reflect.has`), + * not from the grafted APIs. + */ + #lazyData: FactoryOut | undefined = undefined; + + readonly #kernelArgs: KernelArgs; + + /** + * @param id - String identifier. + * @param runtime - Data store runtime. + * @param attributes - The attributes for the map. + */ + public constructor( + id: string, + runtime: IFluidDataStoreRuntime, + attributes: IChannelAttributes, + public readonly factory: SharedKernelFactory, + telemetryContextPrefix: string, + ) { + super(id, runtime, attributes, telemetryContextPrefix); + + this.#kernelArgs = { + sharedObject: this, + serializer: this.serializer, + submitLocalMessage: (op, localOpMetadata) => + this.submitLocalMessage(op, localOpMetadata), + eventEmitter: this, + logger: this.logger, + idCompressor: runtime.idCompressor, + lastSequenceNumber: () => this.deltaManager.lastSequenceNumber, + }; + } + + protected override summarizeCore( + serializer: IFluidSerializer, + telemetryContext?: ITelemetryContext, + incrementalSummaryContext?: IExperimentalIncrementalSummaryContext, + ): ISummaryTreeWithStats { + return this.#kernel.summarizeCore(serializer, telemetryContext, incrementalSummaryContext); + } + + protected override initializeLocalCore(): void { + this.#initializeData(this.factory.create(this.#kernelArgs)); + } + + #initializeData(data: FactoryOut): void { + assert(this.#lazyData === undefined, "initializeData must be called first and only once"); + this.#lazyData = data; + + // Properties of ISharedObject that TOut is likely to implement by forwarding to `this`. + const skipForwarding: Set = new Set([ + "IFluidLoadable", + "isAttached", + "id", + "handle", + "attributes", + ]); + + // Do some sanity checks that the properties above, if present on thew view, match the properties on this. + { + const d = data as Partial; + assert( + d.IFluidLoadable === undefined || d.IFluidLoadable === this.IFluidLoadable, + "must match if provided", + ); + // Not fully robust, but as much validation as is practical. + assert( + d.isAttached === undefined || d.isAttached() === this.isAttached(), + "must match if provided", + ); + assert(d.id === undefined || d.id === this.id, "must match if provided"); + assert(d.handle === undefined || d.handle === this.handle, "must match if provided"); + assert( + d.attributes === undefined || d.attributes === this.attributes, + "must match if provided", + ); + } + + // Make `this` implement TOut. + mergeAPIs(this, data.view, skipForwarding); + } + + get #kernel(): SharedKernel { + return (this.#lazyData ?? fail("must initializeData first")).kernel; + } + + protected override async loadCore(storage: IChannelStorageService): Promise { + this.#initializeData(await this.factory.loadCore(this.#kernelArgs, storage)); + } + + protected override onDisconnect(): void { + this.#kernel.onDisconnect(); + } + + protected override reSubmitCore(content: unknown, localOpMetadata: unknown): void { + this.#kernel.reSubmitCore(content, localOpMetadata); + } + + protected override applyStashedOp(content: unknown): void { + this.#kernel.applyStashedOp(content); + } + + protected override processCore(): void { + fail("processCore should not be called"); + } + + protected override processMessagesCore(messagesCollection: IRuntimeMessageCollection): void { + this.#kernel.processMessagesCore(messagesCollection); + } + + protected override rollback(content: unknown, localOpMetadata: unknown): void { + if (this.#kernel.rollback === undefined) { + super.rollback(content, localOpMetadata); + } else { + this.#kernel.rollback(content, localOpMetadata); + } + } + + protected override didAttach(): void { + this.#kernel.didAttach?.(); + } +} + +/** + * When present on a method, it indicates the methods return value should be replaced with `this` (the wrapper) + * when wrapping the object with the method. + * @internal + */ +export const thisWrap: unique symbol = Symbol("selfWrap"); + +/** + * @internal + */ +export interface FactoryOut { + readonly kernel: SharedKernel; + readonly view: T; +} + +/** + * @internal + */ +export interface SharedKernelFactory { + create(args: KernelArgs): FactoryOut; + + /** + * Create combined with {@link SharedObjectCore.loadCore}. + */ + loadCore(args: KernelArgs, storage: IChannelStorageService): Promise>; +} + +/** + * @internal + */ +export interface KernelArgs { + readonly sharedObject: IChannelView & IFluidLoadable; + readonly serializer: IFluidSerializer; + readonly submitLocalMessage: (op: unknown, localOpMetadata: unknown) => void; + readonly eventEmitter: TypedEventEmitter; + readonly logger: ITelemetryLoggerExt; + readonly idCompressor: IIdCompressor | undefined; + readonly lastSequenceNumber: () => number; +} + +/** + * Add getters to `base` which forward own properties from `extra`. + * @remarks + * This only handles use of "get" and "has": + * therefor APIs involving setting properties should not be used as `Extra`. + * + * Functions from `extra` are bound to the `extra` object and support {@link thisWrap}. + * + * When properties collide, some heuristics are used to determine if its valid, or to assert. + * @internal + */ +export function mergeAPIs( + base: Base, + extra: Extra, + skip: Set = new Set(), +): asserts base is Base & Extra { + for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(extra))) { + if (skip.has(key)) { + continue; + } + if (Reflect.has(base, key)) { + // If the property is on both base and extra, do some validation to attempt to detect if they are clearly incompatible. + // These asserts cover cases currently not required, and likely to cause issues if used. + // They can be relaxed as needed with care. + + const baseDescriptor = Object.getOwnPropertyDescriptor(base, key); + assert(baseDescriptor !== undefined, "merged properties must be own properties"); + assert( + baseDescriptor.set === undefined && baseDescriptor.get === undefined, + "incompatible properties", + ); + assert(descriptor.get === undefined, "incompatible properties"); + assert(baseDescriptor.value === descriptor.value, "incompatible values"); + assert( + baseDescriptor.enumerable === descriptor.enumerable, + "incompatible enumerability", + ); + } + + let getter: () => unknown; + // Bind functions to the extra object and handle thisWrap. + if (typeof descriptor.value === "function") { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const fromExtra: () => Extra | Base = descriptor.value; + getter = () => applyThisWrap(fromExtra, extra, base); + } else { + getter = () => extra[key]; + // If setters become required, support them here. + assert(descriptor.set === undefined, "setters not supported"); + } + + Object.defineProperty(base, key, { + configurable: false, + enumerable: descriptor.enumerable, + get: getter, + // Apply some restrictions preventing cases which are not expected to be needed + // This can catch some cases where base uses this property, but it hasn't been set yet. + }); + } +} + +function applyThisWrap( + f: (...args: TArgs) => TReturn, + oldThis: TReturn, + newThis: TReturn, +): (...args: TArgs) => TReturn { + // eslint-disable-next-line unicorn/prefer-ternary + if (thisWrap in f) { + return (...args: TArgs) => { + const result = f.call(oldThis, ...args); + assert(result === oldThis, "methods returning thisWrap should return this"); + return newThis; + }; + } else { + return f.bind(oldThis); + } +} + +/** + * Options for creating a {@link SharedObjectKind} via {@link makeSharedObjectKind}. + * @typeParam T - The type of the object exposed to the app. + * This can optionally include members from {@link ISharedObject} which will be provided automatically. + * @internal + */ +export interface SharedObjectOptions { + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory."type"} + */ + readonly type: string; + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.attributes} + */ + readonly attributes: IChannelAttributes; + + /** + * The factory used to create the kernel and its view. + * @remarks + * The view produced by this factory will be grafted onto the {@link SharedObject} using {@link mergeAPIs}. + * See {@link mergeAPIs} for more the limitation this applies. + */ + readonly factory: SharedKernelFactory>; + + /** + * {@inheritDoc SharedObject.telemetryContextPrefix} + */ + readonly telemetryContextPrefix: string; +} + +/** + * Utility to create a IChannelFactory classes. + * @remarks + * Prefer using {@link makeSharedObjectKind} instead of exposing the factory is not needed for legacy API compatibility. + * @internal + */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +function makeChannelFactory(options: SharedObjectOptions) { + class ChannelFactory implements IChannelFactory { + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory."type"} + */ + public static readonly Type = options.type; + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.attributes} + */ + public static readonly Attributes: IChannelAttributes = options.attributes; + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory."type"} + */ + public get type(): string { + return ChannelFactory.Type; + } + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.attributes} + */ + public get attributes(): IChannelAttributes { + return ChannelFactory.Attributes; + } + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.load} + */ + public async load( + runtime: IFluidDataStoreRuntime, + id: string, + services: IChannelServices, + attributes: IChannelAttributes, + ): Promise { + const shared = new SharedObjectFromKernel( + id, + runtime, + attributes, + options.factory, + options.telemetryContextPrefix, + ); + await shared.load(services); + return shared as unknown as T & IChannel; + } + + /** + * {@inheritDoc @fluidframework/datastore-definitions#IChannelFactory.create} + */ + public create(runtime: IFluidDataStoreRuntime, id: string): T & IChannel { + const shared = new SharedObjectFromKernel( + id, + runtime, + ChannelFactory.Attributes, + options.factory, + options.telemetryContextPrefix, + ); + + shared.initializeLocal(); + + return shared as unknown as T & IChannel; + } + } + + return ChannelFactory; +} + +/** + * Utility to create a {@link SharedObjectKind}. + * @privateRemarks + * Using this API avoids having to subclasses any Fluid Framework types, + * reducing the coupling between the framework and the SharedObject implementation. + * @internal + */ +export function makeSharedObjectKind( + options: SharedObjectOptions, +): ISharedObjectKind & SharedObjectKind { + return createSharedObjectKind(makeChannelFactory(options)); +} diff --git a/packages/dds/tree/src/shared-tree/sharedTree.ts b/packages/dds/tree/src/shared-tree/sharedTree.ts index a23d9ff617f7..568945135bf0 100644 --- a/packages/dds/tree/src/shared-tree/sharedTree.ts +++ b/packages/dds/tree/src/shared-tree/sharedTree.ts @@ -13,6 +13,7 @@ import type { IChannelStorageService } from "@fluidframework/datastore-definitio import type { IChannelView, IFluidSerializer, + SharedKernel, } from "@fluidframework/shared-object-base/internal"; import { UsageError, @@ -192,12 +193,24 @@ function getCodecVersions(formatVersion: number): ExplicitCodecVersions { * TODO: detail compatibility requirements. */ @breakingClass -export class SharedTreeKernel extends SharedTreeCore { +export class SharedTreeKernel + extends SharedTreeCore + implements SharedKernel +{ public readonly checkout: TreeCheckout; public get storedSchema(): TreeStoredSchemaRepository { return this.checkout.storedSchema; } + /** + * The app facing API for SharedTree implemented by this Kernel. + * @remarks + * This is the API grafted onto the ISharedObject which apps can access. + * It includes both the APIs used for internal testing, and public facing APIs (both stable and unstable). + * Different users will have access to different subsets of this API, see {@link ITree}, {@link ITreeAlpha} and {@link ITreeInternal} which this {@link ITreePrivate} extends. + */ + public readonly view: ITreePrivate; + public constructor( breaker: Breakable, sharedObject: IChannelView & IFluidLoadable, @@ -331,6 +344,19 @@ export class SharedTreeKernel extends SharedTreeCore this.contentSnapshot(), + exportSimpleSchema: () => this.exportSimpleSchema(), + exportVerbose: () => this.exportVerbose(), + viewWith: this.viewWith.bind(this), + handle: sharedObject.handle, + IFluidLoadable: sharedObject, + id: sharedObject.id, + attributes: sharedObject.attributes, + isAttached: () => sharedObject.isAttached(), + kernel: this, + }; } public exportVerbose(): VerboseTree | undefined { diff --git a/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts b/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts index 0bcef8f22af2..397d1d43affd 100644 --- a/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/sharedTree.spec.ts @@ -94,10 +94,9 @@ import { SharedTree as SharedTreeKind, type ISharedTree, } from "../../treeFactory.js"; -import { - SharedObjectCore, - type ISharedObjectKind, - type SharedObjectKind, +import type { + ISharedObjectKind, + SharedObjectKind, } from "@fluidframework/shared-object-base/internal"; import { TestAnchor } from "../testAnchor.js"; // eslint-disable-next-line import/no-internal-modules @@ -736,12 +735,15 @@ describe("SharedTree", () => { getBranch(tree).branch(); view.root.insertAtEnd("b"); - const tree2 = sharedTreeFactory.create(runtime, "tree2"); - assert(tree2 instanceof SharedObjectCore); - await tree2.load({ - deltaConnection: runtime.createDeltaConnection(), - objectStorage: MockStorage.createFromSummary((await tree.summarize()).summary), - }); + const tree2 = await sharedTreeFactory.load( + runtime, + "tree2", + { + deltaConnection: runtime.createDeltaConnection(), + objectStorage: MockStorage.createFromSummary((await tree.summarize()).summary), + }, + sharedTreeFactory.attributes, + ); const loadedView = tree2.viewWith( new TreeViewConfiguration({ diff --git a/packages/dds/tree/src/treeFactory.ts b/packages/dds/tree/src/treeFactory.ts index 223497acee68..fc146890c84c 100644 --- a/packages/dds/tree/src/treeFactory.ts +++ b/packages/dds/tree/src/treeFactory.ts @@ -3,55 +3,30 @@ * Licensed under the MIT License. */ -import type { - IChannelAttributes, - IChannelFactory, - IFluidDataStoreRuntime, - IChannelServices, - IChannelStorageService, -} from "@fluidframework/datastore-definitions/internal"; -import type { - ITelemetryContext, - IExperimentalIncrementalSummaryContext, - ISummaryTreeWithStats, - IRuntimeMessageCollection, -} from "@fluidframework/runtime-definitions/internal"; +import type { IChannelStorageService } from "@fluidframework/datastore-definitions/internal"; -import type { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; import type { SharedObjectKind } from "@fluidframework/shared-object-base"; import { - type IFluidSerializer, type ISharedObject, type ISharedObjectKind, - SharedObject, - createSharedObjectKind, + makeSharedObjectKind, + type KernelArgs, + type SharedKernelFactory, + type SharedObjectOptions, + type FactoryOut, } from "@fluidframework/shared-object-base/internal"; -import type { - SchematizingSimpleTreeView, - SharedTreeContentSnapshot, - SharedTreeOptions, - SharedTreeOptionsInternal, - SharedTreeEditBuilder, - SharedTreeChange, - ITreePrivate, +import { + SharedTreeKernel, + type ITreePrivate, + type SharedTreeOptions, + type SharedTreeOptionsInternal, } from "./shared-tree/index.js"; -import type { - ImplicitFieldSchema, - ITree, - ReadSchema, - SimpleTreeSchema, - TreeView, - TreeViewConfiguration, - UnsafeUnknownSchema, - VerboseTree, -} from "./simple-tree/index.js"; -import { SharedTreeFactoryType, SharedTreeAttributes } from "./sharedTreeAttributes.js"; +import type { ITree } from "./simple-tree/index.js"; + import { Breakable } from "./util/index.js"; -import { SharedTreeKernel } from "./shared-tree/index.js"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import { fail } from "@fluidframework/core-utils/internal"; -import type { SharedTreeCore } from "./shared-tree-core/index.js"; +import { SharedTreeFactoryType, SharedTreeAttributes } from "./sharedTreeAttributes.js"; /** * {@link ITreePrivate} extended with ISharedObject. @@ -61,140 +36,45 @@ import type { SharedTreeCore } from "./shared-tree-core/index.js"; export interface ISharedTree extends ISharedObject, ITreePrivate {} /** - * Shared object wrapping {@link SharedTreeKernel}. + * Creates a factory for shared tree kernels with the given options. + * @remarks + * Exposes {@link ITreePrivate} to allow access to internals in tests without a cast. + * Code exposing this beyond this package will need to update to a more public type. */ -class SharedTreeImpl extends SharedObject implements ISharedTree { - private readonly breaker: Breakable = new Breakable("Shared Tree"); - - public readonly kernel: SharedTreeKernel; - - public constructor( - id: string, - runtime: IFluidDataStoreRuntime, - attributes: IChannelAttributes, - optionsParam: SharedTreeOptionsInternal, - telemetryContextPrefix: string = "fluid_sharedTree_", - ) { - super(id, runtime, attributes, telemetryContextPrefix); - if (runtime.idCompressor === undefined) { +function treeKernelFactory( + options: SharedTreeOptionsInternal, +): SharedKernelFactory { + function treeFromKernelArgs(args: KernelArgs): SharedTreeKernel { + if (args.idCompressor === undefined) { throw new UsageError("IdCompressor must be enabled to use SharedTree"); } - this.kernel = new SharedTreeKernel( - this.breaker, - this, - this.serializer, - (content, localOpMetadata) => this.submitLocalMessage(content, localOpMetadata), - () => this.deltaManager.lastSequenceNumber, - this.logger, - runtime.idCompressor, - optionsParam, + return new SharedTreeKernel( + new Breakable("SharedTree"), + args.sharedObject, + args.serializer, + args.submitLocalMessage, + args.lastSequenceNumber, + args.logger, + args.idCompressor, + options, ); } - public summarizeCore( - serializer: IFluidSerializer, - telemetryContext?: ITelemetryContext, - incrementalSummaryContext?: IExperimentalIncrementalSummaryContext, - ): ISummaryTreeWithStats { - return this.kernel.summarizeCore(serializer, telemetryContext, incrementalSummaryContext); - } - - protected processCore( - message: ISequencedDocumentMessage, - local: boolean, - localOpMetadata: unknown, - ): void { - fail(0xb75 /* processCore should not be called on SharedTree */); - } - - protected override processMessagesCore(messagesCollection: IRuntimeMessageCollection): void { - this.kernel.processMessagesCore(messagesCollection); - } - - protected onDisconnect(): void { - this.kernel.onDisconnect(); - } - - public exportVerbose(): VerboseTree | undefined { - return this.kernel.exportVerbose(); - } - - public exportSimpleSchema(): SimpleTreeSchema { - return this.kernel.exportSimpleSchema(); - } - - public contentSnapshot(): SharedTreeContentSnapshot { - return this.kernel.contentSnapshot(); - } - - // For the new TreeViewAlpha API - public viewWith( - config: TreeViewConfiguration>, - ): SchematizingSimpleTreeView & TreeView>; - - // For the old TreeView API - public viewWith( - config: TreeViewConfiguration, - ): SchematizingSimpleTreeView & TreeView; - - public viewWith( - config: TreeViewConfiguration>, - ): SchematizingSimpleTreeView & TreeView> { - return this.kernel.viewWith(config); - } - - protected override async loadCore(services: IChannelStorageService): Promise { - await this.kernel.loadCore(services); - } - - protected override didAttach(): void { - this.kernel.didAttach(); - } - - protected override applyStashedOp( - ...args: Parameters< - SharedTreeCore["applyStashedOp"] - > - ): void { - this.kernel.applyStashedOp(...args); - } - - protected override reSubmitCore( - ...args: Parameters< - SharedTreeCore["reSubmitCore"] - > - ): void { - this.kernel.reSubmitCore(...args); - } -} - -/** - * A channel factory that creates an {@link ITree}. - */ -class TreeFactory implements IChannelFactory { - public static Type: string = SharedTreeFactoryType; - public readonly type: string = SharedTreeFactoryType; - - public readonly attributes: IChannelAttributes = SharedTreeAttributes; - - public constructor(private readonly options: SharedTreeOptionsInternal) {} - - public async load( - runtime: IFluidDataStoreRuntime, - id: string, - services: IChannelServices, - channelAttributes: Readonly, - ): Promise { - const tree = new SharedTreeImpl(id, runtime, channelAttributes, this.options); - await tree.load(services); - return tree; - } - - public create(runtime: IFluidDataStoreRuntime, id: string): ISharedTree { - const tree = new SharedTreeImpl(id, runtime, this.attributes, this.options); - tree.initializeLocal(); - return tree; - } + return { + create: (args: KernelArgs): FactoryOut => { + const k = treeFromKernelArgs(args); + return { kernel: k, view: k.view }; + }, + + async loadCore( + args: KernelArgs, + storage: IChannelStorageService, + ): Promise> { + const k = treeFromKernelArgs(args); + await k.loadCore(storage); + return { kernel: k, view: k.view }; + }, + }; } /** @@ -234,10 +114,12 @@ export const SharedTree = configuredSharedTree({}); export function configuredSharedTree( options: SharedTreeOptions, ): ISharedObjectKind & SharedObjectKind { - class ConfiguredFactory extends TreeFactory { - public constructor() { - super(options); - } - } - return createSharedObjectKind(ConfiguredFactory); + const sharedObjectOptions: SharedObjectOptions = { + type: SharedTreeFactoryType, + attributes: SharedTreeAttributes, + telemetryContextPrefix: "fluid_sharedTree_", + factory: treeKernelFactory(options), + }; + + return makeSharedObjectKind(sharedObjectOptions); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 29e039d4701a..2446b6f0b9dc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8728,6 +8728,9 @@ importers: '@fluidframework/driver-definitions': specifier: workspace:~ version: link:../../common/driver-definitions + '@fluidframework/id-compressor': + specifier: workspace:~ + version: link:../../runtime/id-compressor '@fluidframework/runtime-definitions': specifier: workspace:~ version: link:../../runtime/runtime-definitions @@ -30154,9 +30157,9 @@ snapshots: '@typescript-eslint/parser': 6.7.5(eslint@8.55.0)(typescript@5.4.5) eslint-config-biome: 1.9.4 eslint-config-prettier: 9.0.0(eslint@8.55.0) - eslint-import-resolver-typescript: 3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1)(eslint@8.55.0) + eslint-import-resolver-typescript: 3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0) eslint-plugin-eslint-comments: 3.2.0(eslint@8.55.0) - eslint-plugin-import: eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0) + eslint-plugin-import: eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0) eslint-plugin-jsdoc: 46.8.2(eslint@8.55.0) eslint-plugin-promise: 6.1.1(eslint@8.55.0) eslint-plugin-react: 7.33.2(eslint@8.55.0) @@ -35999,33 +36002,33 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1)(eslint@8.55.0): + eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0): dependencies: '@nolyfill/is-core-module': 1.0.39 debug: 4.4.0(supports-color@8.1.1) enhanced-resolve: 5.17.1 eslint: 8.55.0 - eslint-module-utils: 2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0) + eslint-module-utils: 2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0) fast-glob: 3.3.3 get-tsconfig: 4.10.0 is-bun-module: 1.3.0 is-glob: 4.0.3 optionalDependencies: - eslint-plugin-import: eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0) + eslint-plugin-import: eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0) transitivePeerDependencies: - '@typescript-eslint/parser' - eslint-import-resolver-node - eslint-import-resolver-webpack - supports-color - eslint-module-utils@2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0): + eslint-module-utils@2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0): dependencies: debug: 3.2.7 optionalDependencies: '@typescript-eslint/parser': 6.7.5(eslint@8.55.0)(typescript@5.4.5) eslint: 8.55.0 eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1)(eslint@8.55.0) + eslint-import-resolver-typescript: 3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0) transitivePeerDependencies: - supports-color @@ -36039,13 +36042,13 @@ snapshots: eslint: 8.55.0 ignore: 5.3.2 - eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0): + eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0): dependencies: debug: 4.4.0(supports-color@8.1.1) doctrine: 3.0.0 eslint: 8.55.0 eslint-import-resolver-node: 0.3.9 - eslint-module-utils: 2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.55.0) + eslint-module-utils: 2.12.0(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint-plugin-i@2.29.1(@typescript-eslint/parser@6.7.5(eslint@8.55.0)(typescript@5.4.5))(eslint@8.55.0))(eslint@8.55.0))(eslint@8.55.0) get-tsconfig: 4.10.0 is-glob: 4.0.3 minimatch: 3.1.2 From 2c2af57b44ec0a84c5673b650876e974e5d7b6dc Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 18 Apr 2025 09:23:24 -0700 Subject: [PATCH 2/6] Simplify mergeAPIs, avoid overlap --- .../src/sharedObjectKernel.ts | 56 +------------------ packages/dds/tree/src/shared-tree/index.ts | 1 + .../dds/tree/src/shared-tree/sharedTree.ts | 9 +-- packages/dds/tree/src/treeFactory.ts | 7 ++- 4 files changed, 11 insertions(+), 62 deletions(-) diff --git a/packages/dds/shared-object-base/src/sharedObjectKernel.ts b/packages/dds/shared-object-base/src/sharedObjectKernel.ts index 28534bd0f0c2..939e9ff69512 100644 --- a/packages/dds/shared-object-base/src/sharedObjectKernel.ts +++ b/packages/dds/shared-object-base/src/sharedObjectKernel.ts @@ -162,37 +162,8 @@ class SharedObjectFromKernel< assert(this.#lazyData === undefined, "initializeData must be called first and only once"); this.#lazyData = data; - // Properties of ISharedObject that TOut is likely to implement by forwarding to `this`. - const skipForwarding: Set = new Set([ - "IFluidLoadable", - "isAttached", - "id", - "handle", - "attributes", - ]); - - // Do some sanity checks that the properties above, if present on thew view, match the properties on this. - { - const d = data as Partial; - assert( - d.IFluidLoadable === undefined || d.IFluidLoadable === this.IFluidLoadable, - "must match if provided", - ); - // Not fully robust, but as much validation as is practical. - assert( - d.isAttached === undefined || d.isAttached() === this.isAttached(), - "must match if provided", - ); - assert(d.id === undefined || d.id === this.id, "must match if provided"); - assert(d.handle === undefined || d.handle === this.handle, "must match if provided"); - assert( - d.attributes === undefined || d.attributes === this.attributes, - "must match if provided", - ); - } - // Make `this` implement TOut. - mergeAPIs(this, data.view, skipForwarding); + mergeAPIs(this, data.view); } get #kernel(): SharedKernel { @@ -284,36 +255,15 @@ export interface KernelArgs { * * Functions from `extra` are bound to the `extra` object and support {@link thisWrap}. * - * When properties collide, some heuristics are used to determine if its valid, or to assert. + * When asserts when properties collide. * @internal */ export function mergeAPIs( base: Base, extra: Extra, - skip: Set = new Set(), ): asserts base is Base & Extra { for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(extra))) { - if (skip.has(key)) { - continue; - } - if (Reflect.has(base, key)) { - // If the property is on both base and extra, do some validation to attempt to detect if they are clearly incompatible. - // These asserts cover cases currently not required, and likely to cause issues if used. - // They can be relaxed as needed with care. - - const baseDescriptor = Object.getOwnPropertyDescriptor(base, key); - assert(baseDescriptor !== undefined, "merged properties must be own properties"); - assert( - baseDescriptor.set === undefined && baseDescriptor.get === undefined, - "incompatible properties", - ); - assert(descriptor.get === undefined, "incompatible properties"); - assert(baseDescriptor.value === descriptor.value, "incompatible values"); - assert( - baseDescriptor.enumerable === descriptor.enumerable, - "incompatible enumerability", - ); - } + assert(!Reflect.has(base, key), "colliding properties"); let getter: () => unknown; // Bind functions to the extra object and handle thisWrap. diff --git a/packages/dds/tree/src/shared-tree/index.ts b/packages/dds/tree/src/shared-tree/index.ts index 38ff54b42575..344e357c8816 100644 --- a/packages/dds/tree/src/shared-tree/index.ts +++ b/packages/dds/tree/src/shared-tree/index.ts @@ -21,6 +21,7 @@ export { ForestTypeExpensiveDebug, ForestTypeReference, exportSimpleSchema, + type SharedTreeKernelView, } from "./sharedTree.js"; export { diff --git a/packages/dds/tree/src/shared-tree/sharedTree.ts b/packages/dds/tree/src/shared-tree/sharedTree.ts index 568945135bf0..59cd32017616 100644 --- a/packages/dds/tree/src/shared-tree/sharedTree.ts +++ b/packages/dds/tree/src/shared-tree/sharedTree.ts @@ -187,6 +187,8 @@ function getCodecVersions(formatVersion: number): ExplicitCodecVersions { return versions; } +export type SharedTreeKernelView = Omit; + /** * SharedTreeCore, configured with a good set of indexes and field kinds which will maintain compatibility over time. * @@ -209,7 +211,7 @@ export class SharedTreeKernel * It includes both the APIs used for internal testing, and public facing APIs (both stable and unstable). * Different users will have access to different subsets of this API, see {@link ITree}, {@link ITreeAlpha} and {@link ITreeInternal} which this {@link ITreePrivate} extends. */ - public readonly view: ITreePrivate; + public readonly view: SharedTreeKernelView; public constructor( breaker: Breakable, @@ -350,11 +352,6 @@ export class SharedTreeKernel exportSimpleSchema: () => this.exportSimpleSchema(), exportVerbose: () => this.exportVerbose(), viewWith: this.viewWith.bind(this), - handle: sharedObject.handle, - IFluidLoadable: sharedObject, - id: sharedObject.id, - attributes: sharedObject.attributes, - isAttached: () => sharedObject.isAttached(), kernel: this, }; } diff --git a/packages/dds/tree/src/treeFactory.ts b/packages/dds/tree/src/treeFactory.ts index fc146890c84c..39b7516ee969 100644 --- a/packages/dds/tree/src/treeFactory.ts +++ b/packages/dds/tree/src/treeFactory.ts @@ -21,6 +21,7 @@ import { type ITreePrivate, type SharedTreeOptions, type SharedTreeOptionsInternal, + type SharedTreeKernelView, } from "./shared-tree/index.js"; import type { ITree } from "./simple-tree/index.js"; @@ -43,7 +44,7 @@ export interface ISharedTree extends ISharedObject, ITreePrivate {} */ function treeKernelFactory( options: SharedTreeOptionsInternal, -): SharedKernelFactory { +): SharedKernelFactory { function treeFromKernelArgs(args: KernelArgs): SharedTreeKernel { if (args.idCompressor === undefined) { throw new UsageError("IdCompressor must be enabled to use SharedTree"); @@ -61,7 +62,7 @@ function treeKernelFactory( } return { - create: (args: KernelArgs): FactoryOut => { + create: (args: KernelArgs): FactoryOut => { const k = treeFromKernelArgs(args); return { kernel: k, view: k.view }; }, @@ -69,7 +70,7 @@ function treeKernelFactory( async loadCore( args: KernelArgs, storage: IChannelStorageService, - ): Promise> { + ): Promise> { const k = treeFromKernelArgs(args); await k.loadCore(storage); return { kernel: k, view: k.view }; From c8ed1d326a0f90a8626af7dfb0d771f793b66374 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 18 Apr 2025 10:30:58 -0700 Subject: [PATCH 3/6] More docs --- .../src/sharedObjectKernel.ts | 58 +++++++++++++++++-- .../datastore-definitions.legacy.alpha.api.md | 1 - .../src/dataStoreRuntime.ts | 5 ++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/packages/dds/shared-object-base/src/sharedObjectKernel.ts b/packages/dds/shared-object-base/src/sharedObjectKernel.ts index 939e9ff69512..1f316782995f 100644 --- a/packages/dds/shared-object-base/src/sharedObjectKernel.ts +++ b/packages/dds/shared-object-base/src/sharedObjectKernel.ts @@ -215,6 +215,12 @@ class SharedObjectFromKernel< export const thisWrap: unique symbol = Symbol("selfWrap"); /** + * A {@link SharedKernel} providing the implementation of some distributed data structure (DDS) and the needed runtime facing APIs, + * and a separate view object which exposes the app facing APIs (`T`) + * for reading and writing data which are specific to this particular data structure. + * @remarks + * Output from {@link SharedKernelFactory}. + * This is an alternative to defining DDSs by sub-classing {@link SharedObject}. * @internal */ export interface FactoryOut { @@ -223,6 +229,11 @@ export interface FactoryOut { } /** + * A factory for creating DDSs. + * @remarks + * Outputs {@link FactoryOut}. + * This is an alternative to directly implementing {@link @fluidframework/datastore-definitions#IChannelFactory}. + * Use with {@link makeSharedObjectKind} to create a {@link SharedObjectKind}. * @internal */ export interface SharedKernelFactory { @@ -235,15 +246,40 @@ export interface SharedKernelFactory { } /** + * Inputs for building a {@link SharedKernel} via {@link SharedKernelFactory}. * @internal */ export interface KernelArgs { + /** + * The shared object whose behavior is being implemented. + */ readonly sharedObject: IChannelView & IFluidLoadable; + /** + * {@inheritdoc SharedObject.serializer} + */ readonly serializer: IFluidSerializer; + /** + * {@inheritdoc SharedObjectCore.submitLocalMessage} + */ readonly submitLocalMessage: (op: unknown, localOpMetadata: unknown) => void; + /** + * Top level emitter for events for this object. + * @remarks + * This is needed since the separate kernel and view from {@link FactoryOut} currently have to be recombined, + * and having this as its own thing helps accomplish that. + */ readonly eventEmitter: TypedEventEmitter; + /** + * {@inheritdoc SharedObjectCore.logger} + */ readonly logger: ITelemetryLoggerExt; + /** + * {@inheritdoc @fluidframework/datastore-definitions#IFluidDataStoreRuntime.idCompressor} + */ readonly idCompressor: IIdCompressor | undefined; + /** + * {@inheritdoc @fluidframework/container-definitions#IDeltaManager.lastSequenceNumber} + */ readonly lastSequenceNumber: () => number; } @@ -265,29 +301,39 @@ export function mergeAPIs for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(extra))) { assert(!Reflect.has(base, key), "colliding properties"); + // Detect and special case functions. + // Currently this is done eagerly (when mergeAPIs is called) rather than lazily (when the property is read): + // this eager approach should result in slightly better performance, + // but if functions on `extra` are reassigned over time it will produce incorrect behavior. + // If this functionality is required, the design can be changed. let getter: () => unknown; // Bind functions to the extra object and handle thisWrap. if (typeof descriptor.value === "function") { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const fromExtra: () => Extra | Base = descriptor.value; - getter = () => applyThisWrap(fromExtra, extra, base); + getter = () => forwardMethod(fromExtra, extra, base); + // To catch (and error on) cases where the function is reassigned and this this eager binding approach is not appropriate, make it non-writable. + Object.defineProperty(extra, key, { ...descriptor, writable: false }); } else { getter = () => extra[key]; - // If setters become required, support them here. - assert(descriptor.set === undefined, "setters not supported"); } Object.defineProperty(base, key, { configurable: false, enumerable: descriptor.enumerable, get: getter, - // Apply some restrictions preventing cases which are not expected to be needed - // This can catch some cases where base uses this property, but it hasn't been set yet. + // If setters become required, support them here. }); } } -function applyThisWrap( +/** + * Wrap a method `f` of `oldThis` to be a method of `newThis`. + * @remarks + * The wrapped function will be called with `oldThis` as the `this` parameter. + * It also accounts for when `f` is marked with {@link thisWrap}. + */ +function forwardMethod( f: (...args: TArgs) => TReturn, oldThis: TReturn, newThis: TReturn, diff --git a/packages/runtime/datastore-definitions/api-report/datastore-definitions.legacy.alpha.api.md b/packages/runtime/datastore-definitions/api-report/datastore-definitions.legacy.alpha.api.md index 248e1cf99128..a3ef3c877e1c 100644 --- a/packages/runtime/datastore-definitions/api-report/datastore-definitions.legacy.alpha.api.md +++ b/packages/runtime/datastore-definitions/api-report/datastore-definitions.legacy.alpha.api.md @@ -87,7 +87,6 @@ export interface IFluidDataStoreRuntime extends IEventProvider Date: Mon, 28 Apr 2025 11:05:53 -0700 Subject: [PATCH 4/6] Update packages/runtime/datastore-definitions/src/dataStoreRuntime.ts Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> --- .../runtime/datastore-definitions/src/dataStoreRuntime.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/runtime/datastore-definitions/src/dataStoreRuntime.ts b/packages/runtime/datastore-definitions/src/dataStoreRuntime.ts index e6b05e320103..bcd50b8cca0b 100644 --- a/packages/runtime/datastore-definitions/src/dataStoreRuntime.ts +++ b/packages/runtime/datastore-definitions/src/dataStoreRuntime.ts @@ -77,8 +77,9 @@ export interface IFluidDataStoreRuntime readonly attachState: AttachState; /** - * An optional id compressor. - * When provided, can be used to compress and decompress ids stored in this datastore. + * An optional ID compressor. + * @remarks + * When provided, can be used to compress and decompress IDs stored in this datastore. * Some SharedObjects, like SharedTree, require this. */ readonly idCompressor: IIdCompressor | undefined; From b4e80b08489c0add3aa35433a4ebaedcbca877a6 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Thu, 1 May 2025 16:11:30 -0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> --- .../dds/shared-object-base/src/sharedObjectKernel.ts | 11 ++++++----- packages/dds/tree/src/shared-tree/sharedTree.ts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/dds/shared-object-base/src/sharedObjectKernel.ts b/packages/dds/shared-object-base/src/sharedObjectKernel.ts index 1f316782995f..1a2f666c9afd 100644 --- a/packages/dds/shared-object-base/src/sharedObjectKernel.ts +++ b/packages/dds/shared-object-base/src/sharedObjectKernel.ts @@ -39,6 +39,7 @@ import type { IChannelView } from "./utils.js"; * SharedObjects expose APIs for two consumers: * * 1. The runtime, which uses the SharedObject to summarize, load and apply ops. + * * 2. The app, who uses the SharedObject to read and write data. * * There is some common functionality all shared objects use, provided by {@link SharedObject}. @@ -97,7 +98,7 @@ export interface SharedKernel { /** * SharedObject implementation that delegates to a SharedKernel. * @typeParam TOut - The type of the object exposed to the app. - * Once initialized instances of this class forward properties to the `TOut` val;ue provided by the factory. + * Once initialized, instances of this class forward properties to the `TOut` value provided by the factory. * See {@link mergeAPIs} for more limitations. * * @remarks @@ -286,12 +287,12 @@ export interface KernelArgs { /** * Add getters to `base` which forward own properties from `extra`. * @remarks - * This only handles use of "get" and "has": - * therefor APIs involving setting properties should not be used as `Extra`. + * This only handles use of "get" and "has". + * Therefore, APIs involving setting properties should not be used as `Extra`. * * Functions from `extra` are bound to the `extra` object and support {@link thisWrap}. * - * When asserts when properties collide. + * Asserts when properties collide. * @internal */ export function mergeAPIs( @@ -312,7 +313,7 @@ export function mergeAPIs // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const fromExtra: () => Extra | Base = descriptor.value; getter = () => forwardMethod(fromExtra, extra, base); - // To catch (and error on) cases where the function is reassigned and this this eager binding approach is not appropriate, make it non-writable. + // To catch (and error on) cases where the function is reassigned and this eager binding approach is not appropriate, make it non-writable. Object.defineProperty(extra, key, { ...descriptor, writable: false }); } else { getter = () => extra[key]; diff --git a/packages/dds/tree/src/shared-tree/sharedTree.ts b/packages/dds/tree/src/shared-tree/sharedTree.ts index 59cd32017616..c0c4df66f69c 100644 --- a/packages/dds/tree/src/shared-tree/sharedTree.ts +++ b/packages/dds/tree/src/shared-tree/sharedTree.ts @@ -205,7 +205,7 @@ export class SharedTreeKernel } /** - * The app facing API for SharedTree implemented by this Kernel. + * The app-facing API for SharedTree implemented by this Kernel. * @remarks * This is the API grafted onto the ISharedObject which apps can access. * It includes both the APIs used for internal testing, and public facing APIs (both stable and unstable). From d5733dc8ac0647584892ba896c12fe9bd30da1b0 Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Fri, 2 May 2025 17:00:38 -0700 Subject: [PATCH 6/6] Update packages/dds/shared-object-base/src/sharedObjectKernel.ts Co-authored-by: Abram Sanderson --- packages/dds/shared-object-base/src/sharedObjectKernel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dds/shared-object-base/src/sharedObjectKernel.ts b/packages/dds/shared-object-base/src/sharedObjectKernel.ts index 1a2f666c9afd..a85aaf04e8ad 100644 --- a/packages/dds/shared-object-base/src/sharedObjectKernel.ts +++ b/packages/dds/shared-object-base/src/sharedObjectKernel.ts @@ -372,7 +372,7 @@ export interface SharedObjectOptions { * The factory used to create the kernel and its view. * @remarks * The view produced by this factory will be grafted onto the {@link SharedObject} using {@link mergeAPIs}. - * See {@link mergeAPIs} for more the limitation this applies. + * See {@link mergeAPIs} for more information on limitations that apply. */ readonly factory: SharedKernelFactory>;