From dbf0696243ecc7149cbef05fca5ca2f07f47a1a2 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Fri, 30 May 2025 10:04:36 -0700 Subject: [PATCH 01/13] improvement(client-presence): use opaque json internally Simplify internal unknown (generic) customer data that must be Json serializable using Opaque Json types At boundaries of API ins and outs, Json filtered types are cast to/from Opaque Json types. See new helpers: - `fullySerializableToOpaqueJson` - `asDeserializedJson` - `asDeeplyReadonlyDeserializedJson` Notifications utilities are updated and removal of `string &` for `NotificationListeners` allowed clean up of `@ts-ignore-error`. `NotificationsManagerImpl`'s `emit` implementation now just types `name` to `string`.' --- .../presence/src/exposedInternalTypes.ts | 5 ++- .../presence/src/exposedUtilityTypes.ts | 19 +++++---- .../framework/presence/src/internalUtils.ts | 41 ++++++++++++++++++- .../presence/src/latestMapValueManager.ts | 24 +++++++---- .../presence/src/latestValueManager.ts | 35 ++++++++++++---- .../presence/src/notificationsManager.ts | 34 +++++++-------- .../framework/presence/src/systemWorkspace.ts | 8 ++-- .../src/test/latestValueManager.spec.ts | 16 ++++++++ 8 files changed, 133 insertions(+), 49 deletions(-) diff --git a/packages/framework/presence/src/exposedInternalTypes.ts b/packages/framework/presence/src/exposedInternalTypes.ts index 46e457098129..28b5af076413 100644 --- a/packages/framework/presence/src/exposedInternalTypes.ts +++ b/packages/framework/presence/src/exposedInternalTypes.ts @@ -6,6 +6,7 @@ import type { JsonDeserialized, JsonSerializable, + OpaqueJsonDeserialized, } from "@fluidframework/core-interfaces/internal/exposedUtilityTypes"; /** @@ -29,14 +30,14 @@ export namespace InternalTypes { * @system */ export interface ValueOptionalState extends ValueStateMetadata { - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } /** * @system */ export interface ValueRequiredState extends ValueStateMetadata { - value: JsonDeserialized; + value: OpaqueJsonDeserialized; } /** diff --git a/packages/framework/presence/src/exposedUtilityTypes.ts b/packages/framework/presence/src/exposedUtilityTypes.ts index a6df51d9f084..603118da484a 100644 --- a/packages/framework/presence/src/exposedUtilityTypes.ts +++ b/packages/framework/presence/src/exposedUtilityTypes.ts @@ -19,18 +19,21 @@ import type { // eslint-disable-next-line @typescript-eslint/no-namespace export namespace InternalUtilityTypes { /** - * `true` iff the given type is an acceptable shape for a notification. + * `IfListener` iff the given type is an acceptable shape for a notification. + * `Else` otherwise. * * @system */ - export type IsNotificationListener = Event extends (...args: infer P) => void + export type IfNotificationListener = Event extends ( + ...args: infer P + ) => void ? CoreInternalUtilityTypes.IfSameType< P, JsonSerializable

& JsonDeserialized

, - true, - false + IfListener, + Else > - : false; + : Else; /** * Used to specify the kinds of notifications emitted by a {@link NotificationListenable}. @@ -52,7 +55,7 @@ export namespace InternalUtilityTypes { * @system */ export type NotificationListeners = { - [P in string & keyof E as IsNotificationListener extends true ? P : never]: E[P]; + [P in keyof E as IfNotificationListener]: E[P]; }; /** @@ -60,7 +63,7 @@ export namespace InternalUtilityTypes { * * @system */ - export type JsonDeserializedParameters any> = T extends ( + export type JsonDeserializedParameters any> = T extends ( ...args: infer P ) => any ? JsonDeserialized

@@ -71,7 +74,7 @@ export namespace InternalUtilityTypes { * * @system */ - export type JsonSerializableParameters any> = T extends ( + export type JsonSerializableParameters any> = T extends ( ...args: infer P ) => any ? JsonSerializable

diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index eb741cbe8e46..8045b3bf4a6a 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -3,7 +3,13 @@ * Licensed under the MIT License. */ -import type { DeepReadonly } from "@fluidframework/core-interfaces/internal"; +import type { + DeepReadonly, + JsonDeserialized, + JsonSerializable, + OpaqueJsonDeserialized, + OpaqueJsonSerializable, +} from "@fluidframework/core-interfaces/internal"; /** * Returns union of types of values in a record. @@ -75,8 +81,39 @@ export function getOrCreateRecord(value: T): DeepReadonly { return value as DeepReadonly; } + +export function asDeeplyReadonlyDeserializedJson( + value: OpaqueJsonDeserialized, +): DeepReadonly>; +export function asDeeplyReadonlyDeserializedJson( + value: OpaqueJsonDeserialized | undefined, +): DeepReadonly> | undefined; +/** + * Does nothing helper to apply deep immutability to a value's opaque Json type revealing the Json type. + */ +export function asDeeplyReadonlyDeserializedJson( + value: OpaqueJsonDeserialized | undefined, +): DeepReadonly> | undefined { + return value as DeepReadonly> | undefined; +} + +/** + * Does nothing helper to reveal the Json type from a value's opaque Json type. + */ +export function asDeserializedJson(value: OpaqueJsonDeserialized): JsonDeserialized { + return value as JsonDeserialized; +} + +/** + * Does nothing helper to automatically cast Json type to Opaque Json type. + */ +export function fullySerializableToOpaqueJson( + value: JsonSerializable & JsonDeserialized, +): OpaqueJsonSerializable & OpaqueJsonDeserialized { + return value as OpaqueJsonSerializable & OpaqueJsonDeserialized; +} diff --git a/packages/framework/presence/src/latestMapValueManager.ts b/packages/framework/presence/src/latestMapValueManager.ts index 44e1adff0f22..1f7e6ae7c22e 100644 --- a/packages/framework/presence/src/latestMapValueManager.ts +++ b/packages/framework/presence/src/latestMapValueManager.ts @@ -16,7 +16,13 @@ import type { BroadcastControls, BroadcastControlSettings } from "./broadcastCon import { OptionalBroadcastControl } from "./broadcastControls.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; -import { asDeeplyReadonly, objectEntries, objectKeys } from "./internalUtils.js"; +import { + asDeeplyReadonly, + objectEntries, + objectKeys, + fullySerializableToOpaqueJson, + asDeeplyReadonlyDeserializedJson, +} from "./internalUtils.js"; import type { LatestClientData, LatestData, LatestMetadata } from "./latestValueTypes.js"; import type { AttendeeId, Attendee, Presence, SpecificAttendee } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; @@ -286,23 +292,24 @@ class ValueMapImpl implements StateMap { ): void { for (const [key, item] of objectEntries(this.value.items)) { if (item.value !== undefined) { - callbackfn(asDeeplyReadonly(item.value), key, this); + callbackfn(asDeeplyReadonlyDeserializedJson(item.value), key, this); } } } public get(key: K): DeepReadonly> | undefined { - return asDeeplyReadonly(this.value.items[key]?.value); + return asDeeplyReadonlyDeserializedJson(this.value.items[key]?.value); } public has(key: K): boolean { return this.value.items[key]?.value !== undefined; } - public set(key: K, value: JsonSerializable & JsonDeserialized): this { + public set(key: K, inValue: JsonSerializable & JsonDeserialized): this { + const value = fullySerializableToOpaqueJson(inValue); if (!(key in this.value.items)) { this.countDefined += 1; this.value.items[key] = { rev: 0, timestamp: 0, value }; } this.updateItem(key, value); - this.emitter.emit("localItemUpdated", { key, value: asDeeplyReadonly(value) }); + this.emitter.emit("localItemUpdated", { key, value: asDeeplyReadonly(inValue) }); return this; } public get size(): number { @@ -433,7 +440,7 @@ class LatestMapRawValueManagerImpl< const value = item.value; if (value !== undefined) { items.set(key, { - value: asDeeplyReadonly(value), + value: asDeeplyReadonlyDeserializedJson(value), metadata: { revision: item.rev, timestamp: item.timestamp }, }); } @@ -484,7 +491,7 @@ class LatestMapRawValueManagerImpl< currentState.items[key] = item; const metadata = { revision: item.rev, timestamp: item.timestamp }; if (item.value !== undefined) { - const itemValue = asDeeplyReadonly(item.value); + const itemValue = asDeeplyReadonlyDeserializedJson(item.value); const updatedItem = { attendee, key, @@ -512,6 +519,7 @@ class LatestMapRawValueManagerImpl< /** * Arguments that are passed to the {@link StateFactory.latestMap} function. * + * @input * @beta */ export interface LatestMapArguments { @@ -559,7 +567,7 @@ export function latestMap< value.items[key] = { rev: 0, timestamp, - value: initialValues[key], + value: fullySerializableToOpaqueJson(initialValues[key]), }; } } diff --git a/packages/framework/presence/src/latestValueManager.ts b/packages/framework/presence/src/latestValueManager.ts index 08d0fc16732c..2e4fa5132779 100644 --- a/packages/framework/presence/src/latestValueManager.ts +++ b/packages/framework/presence/src/latestValueManager.ts @@ -16,7 +16,12 @@ import type { BroadcastControls, BroadcastControlSettings } from "./broadcastCon import { OptionalBroadcastControl } from "./broadcastControls.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; -import { asDeeplyReadonly, objectEntries } from "./internalUtils.js"; +import { + asDeeplyReadonly, + asDeeplyReadonlyDeserializedJson, + fullySerializableToOpaqueJson, + objectEntries, +} from "./internalUtils.js"; import type { LatestClientData, LatestData } from "./latestValueTypes.js"; import type { Attendee, Presence } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; @@ -114,13 +119,13 @@ class LatestValueManagerImpl } public get local(): DeepReadonly> { - return asDeeplyReadonly(this.value.value); + return asDeeplyReadonlyDeserializedJson(this.value.value); } public set local(value: JsonSerializable & JsonDeserialized) { this.value.rev += 1; this.value.timestamp = Date.now(); - this.value.value = value; + this.value.value = fullySerializableToOpaqueJson(value); this.datastore.localUpdate(this.key, this.value, { allowableUpdateLatencyMs: this.controls.allowableUpdateLatencyMs, }); @@ -134,7 +139,7 @@ class LatestValueManagerImpl if (attendeeId !== allKnownStates.self) { yield { attendee: this.datastore.presence.attendees.getAttendee(attendeeId), - value: asDeeplyReadonly(value.value), + value: asDeeplyReadonlyDeserializedJson(value.value), metadata: { revision: value.rev, timestamp: value.timestamp }, }; } @@ -155,7 +160,7 @@ class LatestValueManagerImpl throw new Error("No entry for clientId"); } return { - value: asDeeplyReadonly(clientState.value), + value: asDeeplyReadonlyDeserializedJson(clientState.value), metadata: { revision: clientState.rev, timestamp: Date.now() }, }; } @@ -176,24 +181,35 @@ class LatestValueManagerImpl () => this.events.emit("remoteUpdated", { attendee, - value: asDeeplyReadonly(value.value), + value: asDeeplyReadonlyDeserializedJson(value.value), metadata: { revision: value.rev, timestamp: value.timestamp }, }), ]; } } +/** + * Shallow clone an object that might be null. + * + * @param value - The object to clone + * @returns A shallow clone of the input value + * @internal + */ +export function shallowCloneNullableObject(value: T): T { + return value === null ? value : shallowCloneObject(value); +} + /** * Arguments that are passed to the {@link StateFactory.latest} function. * + * @input * @beta */ export interface LatestArguments { /** * The initial value of the local state. */ - // eslint-disable-next-line @rushstack/no-new-null - local: JsonSerializable & JsonDeserialized & (object | null); + local: JsonSerializable & JsonDeserialized; /** * See {@link BroadcastControlSettings}. @@ -213,10 +229,11 @@ export function latest( // Latest takes ownership of the initial local value but makes a shallow // copy for basic protection. + const opaqueLocal = fullySerializableToOpaqueJson(local); const value: InternalTypes.ValueRequiredState = { rev: 0, timestamp: Date.now(), - value: local === null ? local : shallowCloneObject(local), + value: shallowCloneNullableObject(opaqueLocal), }; const factory = ( key: Key, diff --git a/packages/framework/presence/src/notificationsManager.ts b/packages/framework/presence/src/notificationsManager.ts index 12857b260599..49d2d24c3c52 100644 --- a/packages/framework/presence/src/notificationsManager.ts +++ b/packages/framework/presence/src/notificationsManager.ts @@ -10,6 +10,7 @@ import type { JsonTypeWith } from "@fluidframework/core-interfaces/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { InternalUtilityTypes } from "./exposedUtilityTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; +import { asDeserializedJson, fullySerializableToOpaqueJson } from "./internalUtils.js"; import type { Attendee, PresenceWithNotifications as Presence } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; import { brandIVM } from "./valueManager.js"; @@ -100,7 +101,7 @@ export interface NotificationEmitter>( + broadcast>( notificationName: K, ...args: Parameters ): void; @@ -111,7 +112,7 @@ export interface NotificationEmitter>( + unicast>( notificationName: K, targetAttendee: Attendee, ...args: Parameters @@ -171,26 +172,32 @@ class NotificationsManagerImpl< public readonly events = createEmitter(); public readonly emit: NotificationEmitter = { - broadcast: (name, ...args) => { + broadcast: (name: string, ...args) => { this.datastore.localUpdate( this.key, { rev: 0, timestamp: 0, - value: { name, args: [...(args as JsonTypeWith[])] }, + value: fullySerializableToOpaqueJson({ + name, + args: [...(args as JsonTypeWith[])], + }), ignoreUnmonitored: true, }, // This is a notification, so we want to send it immediately. { allowableUpdateLatencyMs: 0 }, ); }, - unicast: (name, targetAttendee, ...args) => { + unicast: (name: string, targetAttendee, ...args) => { this.datastore.localUpdate( this.key, { rev: 0, timestamp: 0, - value: { name, args: [...(args as JsonTypeWith[])] }, + value: fullySerializableToOpaqueJson({ + name, + args: [...(args as JsonTypeWith[])], + }), ignoreUnmonitored: true, }, // This is a notification, so we want to send it immediately. @@ -202,7 +209,6 @@ class NotificationsManagerImpl< // Workaround for types private readonly notificationsInternal = createEmitter>(); - // @ts-expect-error TODO public readonly notifications: NotificationListenable = this.notificationsInternal; public constructor( @@ -235,25 +241,21 @@ class NotificationsManagerImpl< public update( attendee: Attendee, _received: number, - value: InternalTypes.ValueRequiredState, + updateValue: InternalTypes.ValueRequiredState, ): PostUpdateAction[] { const postUpdateActions: PostUpdateAction[] = []; - const eventName = value.value.name as keyof Listeners>; + const value = asDeserializedJson(updateValue.value); + const eventName = value.name as keyof Listeners>; if (this.notificationsInternal.hasListeners(eventName)) { // Without schema validation, we don't know that the args are the correct type. // For now we assume the user is sending the correct types and there is no corruption along the way. - const args = [attendee, ...value.value.args] as Parameters< + const args = [attendee, ...value.args] as Parameters< NotificationSubscriptions[typeof eventName] >; postUpdateActions.push(() => this.notificationsInternal.emit(eventName, ...args)); } else { postUpdateActions.push(() => - this.events.emit( - "unattendedNotification", - value.value.name, - attendee, - ...value.value.args, - ), + this.events.emit("unattendedNotification", value.name, attendee, ...value.args), ); } return postUpdateActions; diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 342de7818f5e..a483d1869824 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -10,6 +10,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import type { ClientConnectionId } from "./baseTypes.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction } from "./internalTypes.js"; +import { asDeserializedJson, fullySerializableToOpaqueJson } from "./internalUtils.js"; import type { Attendee, AttendeesEvents, AttendeeId, Presence } from "./presence.js"; import { AttendeeStatus } from "./presence.js"; import type { PresenceStatesInternal } from "./presenceStates.js"; @@ -131,7 +132,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { for (const [clientConnectionId, value] of Object.entries( remoteDatastore.clientToSessionId, )) { - const attendeeId = value.value; + const attendeeId = asDeserializedJson(value.value); const { attendee, isJoining } = this.ensureAttendee( attendeeId, clientConnectionId, @@ -146,8 +147,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { postUpdateActions.push(() => this.events.emit("attendeeConnected", attendee)); } - const knownSessionId: InternalTypes.ValueRequiredState | undefined = - this.datastore.clientToSessionId[clientConnectionId]; + const knownSessionId = this.datastore.clientToSessionId[clientConnectionId]; if (knownSessionId === undefined) { this.datastore.clientToSessionId[clientConnectionId] = value; } else { @@ -167,7 +167,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { this.datastore.clientToSessionId[clientConnectionId] = { rev: this.selfAttendee.order++, timestamp: Date.now(), - value: this.selfAttendee.attendeeId, + value: fullySerializableToOpaqueJson(this.selfAttendee.attendeeId), }; // Mark 'Connected' remote attendees connections as stale diff --git a/packages/framework/presence/src/test/latestValueManager.spec.ts b/packages/framework/presence/src/test/latestValueManager.spec.ts index 7afc32adf988..f1cabca79eb2 100644 --- a/packages/framework/presence/src/test/latestValueManager.spec.ts +++ b/packages/framework/presence/src/test/latestValueManager.spec.ts @@ -84,6 +84,22 @@ describe("Presence", () => { assert.deepStrictEqual(workspace.states.nullable.local, null); }); + it("can set and get inferred nullable type as initial value", () => { + // Setup + // Use a function to generate the initial value so that TypeScript + // can't statically infer the type as exactly null. + function generateInitialValue(): { x: number; y: number } | null { + return { x: 0, y: 0 }; + } + const initialValue = generateInitialValue(); + // Act + const workspace = presence.states.getWorkspace(testWorkspaceName, { + nullable: StateFactory.latest({ local: initialValue }), + }); + // Verify + assert.deepStrictEqual(workspace.states.nullable.local, initialValue); + }); + it(".presence provides Presence it was created under", () => { const workspace = presence.states.getWorkspace(testWorkspaceName, { camera: StateFactory.latest({ local: { x: 0, y: 0, z: 0 } }), From a5a09477191b8d2ac3a4990399986240f9bb6dc8 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Mon, 2 Jun 2025 16:18:18 -0700 Subject: [PATCH 02/13] improvement(client-presence): reduce data in requirement to JsonSerializable as we are confident that is sufficient. See #24751. Update opaque cast helper to only require `JsonSerializable`. --- .../presence/src/exposedInternalTypes.ts | 8 ++------ .../presence/src/exposedUtilityTypes.ts | 7 +------ packages/framework/presence/src/internalUtils.ts | 4 ++-- .../presence/src/latestMapValueManager.ts | 16 ++++++++-------- .../framework/presence/src/latestValueManager.ts | 14 +++++++------- .../presence/src/notificationsManager.ts | 6 +++--- .../framework/presence/src/systemWorkspace.ts | 4 ++-- .../presence/src/test/presenceStates.spec.ts | 2 +- 8 files changed, 26 insertions(+), 35 deletions(-) diff --git a/packages/framework/presence/src/exposedInternalTypes.ts b/packages/framework/presence/src/exposedInternalTypes.ts index 28b5af076413..f4df12c4b4be 100644 --- a/packages/framework/presence/src/exposedInternalTypes.ts +++ b/packages/framework/presence/src/exposedInternalTypes.ts @@ -3,11 +3,7 @@ * Licensed under the MIT License. */ -import type { - JsonDeserialized, - JsonSerializable, - OpaqueJsonDeserialized, -} from "@fluidframework/core-interfaces/internal/exposedUtilityTypes"; +import type { OpaqueJsonDeserialized } from "@fluidframework/core-interfaces/internal/exposedUtilityTypes"; /** * Collection of value types that are not intended to be used/imported @@ -122,6 +118,6 @@ export namespace InternalTypes { */ export interface NotificationType { name: string; - args: (JsonSerializable & JsonDeserialized)[]; + args: unknown[]; } } diff --git a/packages/framework/presence/src/exposedUtilityTypes.ts b/packages/framework/presence/src/exposedUtilityTypes.ts index 603118da484a..70b4e9fe7049 100644 --- a/packages/framework/presence/src/exposedUtilityTypes.ts +++ b/packages/framework/presence/src/exposedUtilityTypes.ts @@ -27,12 +27,7 @@ export namespace InternalUtilityTypes { export type IfNotificationListener = Event extends ( ...args: infer P ) => void - ? CoreInternalUtilityTypes.IfSameType< - P, - JsonSerializable

& JsonDeserialized

, - IfListener, - Else - > + ? CoreInternalUtilityTypes.IfSameType, IfListener, Else> : Else; /** diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index 8045b3bf4a6a..27212b7dd32b 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -112,8 +112,8 @@ export function asDeserializedJson(value: OpaqueJsonDeserialized): JsonDes /** * Does nothing helper to automatically cast Json type to Opaque Json type. */ -export function fullySerializableToOpaqueJson( - value: JsonSerializable & JsonDeserialized, +export function serializableToOpaqueJson( + value: JsonSerializable, ): OpaqueJsonSerializable & OpaqueJsonDeserialized { return value as OpaqueJsonSerializable & OpaqueJsonDeserialized; } diff --git a/packages/framework/presence/src/latestMapValueManager.ts b/packages/framework/presence/src/latestMapValueManager.ts index 1f7e6ae7c22e..ce493ac14306 100644 --- a/packages/framework/presence/src/latestMapValueManager.ts +++ b/packages/framework/presence/src/latestMapValueManager.ts @@ -18,10 +18,10 @@ import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; import { asDeeplyReadonly, + asDeeplyReadonlyDeserializedJson, objectEntries, objectKeys, - fullySerializableToOpaqueJson, - asDeeplyReadonlyDeserializedJson, + serializableToOpaqueJson, } from "./internalUtils.js"; import type { LatestClientData, LatestData, LatestMetadata } from "./latestValueTypes.js"; import type { AttendeeId, Attendee, Presence, SpecificAttendee } from "./presence.js"; @@ -128,7 +128,7 @@ export interface LatestMapRawEvents { * @eventProperty */ localItemUpdated: (updatedItem: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; key: K; }) => void; @@ -202,7 +202,7 @@ export interface StateMap { * Make a deep clone before setting, if needed. No comparison is done to detect changes; all * sets are transmitted. */ - set(key: K, value: JsonSerializable & JsonDeserialized): this; + set(key: K, value: JsonSerializable): this; /** * The number of elements in the StateMap. @@ -302,8 +302,8 @@ class ValueMapImpl implements StateMap { public has(key: K): boolean { return this.value.items[key]?.value !== undefined; } - public set(key: K, inValue: JsonSerializable & JsonDeserialized): this { - const value = fullySerializableToOpaqueJson(inValue); + public set(key: K, inValue: JsonSerializable): this { + const value = serializableToOpaqueJson(inValue); if (!(key in this.value.items)) { this.countDefined += 1; this.value.items[key] = { rev: 0, timestamp: 0, value }; @@ -527,7 +527,7 @@ export interface LatestMapArguments & JsonDeserialized; + [K in Keys]: JsonSerializable; }; /** @@ -567,7 +567,7 @@ export function latestMap< value.items[key] = { rev: 0, timestamp, - value: fullySerializableToOpaqueJson(initialValues[key]), + value: serializableToOpaqueJson(initialValues[key]), }; } } diff --git a/packages/framework/presence/src/latestValueManager.ts b/packages/framework/presence/src/latestValueManager.ts index 2e4fa5132779..f94061adcaaf 100644 --- a/packages/framework/presence/src/latestValueManager.ts +++ b/packages/framework/presence/src/latestValueManager.ts @@ -19,8 +19,8 @@ import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; import { asDeeplyReadonly, asDeeplyReadonlyDeserializedJson, - fullySerializableToOpaqueJson, objectEntries, + serializableToOpaqueJson, } from "./internalUtils.js"; import type { LatestClientData, LatestData } from "./latestValueTypes.js"; import type { Attendee, Presence } from "./presence.js"; @@ -47,7 +47,7 @@ export interface LatestRawEvents { * @eventProperty */ localUpdated: (update: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; }) => void; } @@ -83,7 +83,7 @@ export interface LatestRaw { * setting, if needed. No comparison is done to detect changes; all sets are transmitted. */ get local(): DeepReadonly>; - set local(value: JsonSerializable & JsonDeserialized); + set local(value: JsonSerializable); /** * Iterable access to remote clients' values. @@ -122,10 +122,10 @@ class LatestValueManagerImpl return asDeeplyReadonlyDeserializedJson(this.value.value); } - public set local(value: JsonSerializable & JsonDeserialized) { + public set local(value: JsonSerializable) { this.value.rev += 1; this.value.timestamp = Date.now(); - this.value.value = fullySerializableToOpaqueJson(value); + this.value.value = serializableToOpaqueJson(value); this.datastore.localUpdate(this.key, this.value, { allowableUpdateLatencyMs: this.controls.allowableUpdateLatencyMs, }); @@ -209,7 +209,7 @@ export interface LatestArguments { /** * The initial value of the local state. */ - local: JsonSerializable & JsonDeserialized; + local: JsonSerializable; /** * See {@link BroadcastControlSettings}. @@ -229,7 +229,7 @@ export function latest( // Latest takes ownership of the initial local value but makes a shallow // copy for basic protection. - const opaqueLocal = fullySerializableToOpaqueJson(local); + const opaqueLocal = serializableToOpaqueJson(local); const value: InternalTypes.ValueRequiredState = { rev: 0, timestamp: Date.now(), diff --git a/packages/framework/presence/src/notificationsManager.ts b/packages/framework/presence/src/notificationsManager.ts index 49d2d24c3c52..0cff2ee1adb4 100644 --- a/packages/framework/presence/src/notificationsManager.ts +++ b/packages/framework/presence/src/notificationsManager.ts @@ -10,7 +10,7 @@ import type { JsonTypeWith } from "@fluidframework/core-interfaces/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { InternalUtilityTypes } from "./exposedUtilityTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; -import { asDeserializedJson, fullySerializableToOpaqueJson } from "./internalUtils.js"; +import { asDeserializedJson, serializableToOpaqueJson } from "./internalUtils.js"; import type { Attendee, PresenceWithNotifications as Presence } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; import { brandIVM } from "./valueManager.js"; @@ -178,7 +178,7 @@ class NotificationsManagerImpl< { rev: 0, timestamp: 0, - value: fullySerializableToOpaqueJson({ + value: serializableToOpaqueJson({ name, args: [...(args as JsonTypeWith[])], }), @@ -194,7 +194,7 @@ class NotificationsManagerImpl< { rev: 0, timestamp: 0, - value: fullySerializableToOpaqueJson({ + value: serializableToOpaqueJson({ name, args: [...(args as JsonTypeWith[])], }), diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index a483d1869824..5e814cab94e1 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -10,7 +10,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import type { ClientConnectionId } from "./baseTypes.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction } from "./internalTypes.js"; -import { asDeserializedJson, fullySerializableToOpaqueJson } from "./internalUtils.js"; +import { asDeserializedJson, serializableToOpaqueJson } from "./internalUtils.js"; import type { Attendee, AttendeesEvents, AttendeeId, Presence } from "./presence.js"; import { AttendeeStatus } from "./presence.js"; import type { PresenceStatesInternal } from "./presenceStates.js"; @@ -167,7 +167,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { this.datastore.clientToSessionId[clientConnectionId] = { rev: this.selfAttendee.order++, timestamp: Date.now(), - value: fullySerializableToOpaqueJson(this.selfAttendee.attendeeId), + value: serializableToOpaqueJson(this.selfAttendee.attendeeId), }; // Mark 'Connected' remote attendees connections as stale diff --git a/packages/framework/presence/src/test/presenceStates.spec.ts b/packages/framework/presence/src/test/presenceStates.spec.ts index 1aa11dd87885..4268aed0be6e 100644 --- a/packages/framework/presence/src/test/presenceStates.spec.ts +++ b/packages/framework/presence/src/test/presenceStates.spec.ts @@ -43,7 +43,7 @@ describe("Presence", () => { }); declare function createValueManager( - initial: JsonSerializable & JsonDeserialized, + initial: JsonSerializable, ): { instanceBase: new () => unknown } & (( key: Key, datastoreHandle: InternalTypes.StateDatastoreHandle< From 37eb489ec69786b3e063c40037142d8f2f298c85 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Mon, 2 Jun 2025 16:46:45 -0700 Subject: [PATCH 03/13] test(client-presence): updates for Opaque Json use Note: `testUtils.ts`'s `prepareConnectedPresence` has error where an `InboundClientJoinMessage` is supposed to satisfy `OutboundClientJoinMessage`. --- .../presence/src/test/batching.spec.ts | 129 ++++++++++-------- .../presence/src/test/eventing.spec.ts | 3 +- .../src/test/notificationsManager.spec.ts | 49 +++++-- .../src/test/presenceDatastoreManager.spec.ts | 15 +- .../presence/src/test/presenceManager.spec.ts | 7 +- .../framework/presence/src/test/testUtils.ts | 3 +- 6 files changed, 124 insertions(+), 82 deletions(-) diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index fa74fefc29c5..b1e2b155d894 100644 --- a/packages/framework/presence/src/test/batching.spec.ts +++ b/packages/framework/presence/src/test/batching.spec.ts @@ -9,6 +9,7 @@ import { useFakeTimers, type SinonFakeTimers } from "sinon"; import type { NotificationsWorkspace } from "../index.js"; import { Notifications, StateFactory } from "../index.js"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -69,7 +70,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -78,9 +79,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": serializableToOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -100,7 +101,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -109,9 +110,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": { + "value": serializableToOpaqueJson({ "num": 42, - }, + }), }, }, }, @@ -131,7 +132,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -140,9 +141,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1020, - "value": { + "value": serializableToOpaqueJson({ "num": 84, - }, + }), }, }, }, @@ -187,7 +188,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -196,9 +197,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": serializableToOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -235,7 +236,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -244,9 +245,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1060, - "value": { + "value": serializableToOpaqueJson({ "num": 22, - }, + }), }, }, }, @@ -266,7 +267,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -275,9 +276,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 6, "timestamp": 1140, - "value": { + "value": serializableToOpaqueJson({ "num": 90, - }, + }), }, }, }, @@ -343,7 +344,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -352,9 +353,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": { + "value": serializableToOpaqueJson({ "num": 34, - }, + }), }, }, }, @@ -374,7 +375,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -383,9 +384,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 5, "timestamp": 1220, - "value": { + "value": serializableToOpaqueJson({ "num": 90, - }, + }), }, }, }, @@ -449,7 +450,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -458,18 +459,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": serializableToOpaqueJson({ "num": 0, - }, + }), }, }, "immediateUpdate": { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": serializableToOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -489,7 +490,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -498,18 +499,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": { + "value": serializableToOpaqueJson({ "num": 34, - }, + }), }, }, "immediateUpdate": { [attendeeId2]: { "rev": 1, "timestamp": 1110, - "value": { + "value": serializableToOpaqueJson({ "num": 56, - }, + }), }, }, }, @@ -563,7 +564,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -572,18 +573,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": { + "value": serializableToOpaqueJson({ "num": 34, - }, + }), }, }, "note": { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": { + "value": serializableToOpaqueJson({ "message": "will be queued", - }, + }), }, }, }, @@ -600,7 +601,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId2), + }, }, }, "s:name:testStateWorkspace": { @@ -608,7 +613,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": { "message": "final message" }, + "value": serializableToOpaqueJson({ "message": "final message" }), }, }, }, @@ -667,7 +672,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -676,9 +681,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": { + "value": serializableToOpaqueJson({ "num": 34, - }, + }), }, }, }, @@ -687,9 +692,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": { + "value": serializableToOpaqueJson({ "message": "final message", - }, + }), }, }, }, @@ -747,7 +752,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId2), + }, }, }, "n:name:testNotificationWorkspace": { @@ -755,7 +764,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [77] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [77] }), "ignoreUnmonitored": true, }, }, @@ -773,7 +782,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId2), + }, }, }, "n:name:testNotificationWorkspace": { @@ -781,7 +794,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [88] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [88] }), "ignoreUnmonitored": true, }, }, @@ -838,7 +851,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -847,9 +860,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1040, - "value": { + "value": serializableToOpaqueJson({ "num": 56, - }, + }), }, }, }, @@ -858,10 +871,10 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { + "value": serializableToOpaqueJson({ "name": "newId", "args": [99], - }, + }), "ignoreUnmonitored": true, }, }, @@ -882,7 +895,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -891,10 +904,10 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { + "value": serializableToOpaqueJson({ "name": "newId", "args": [111], - }, + }), "ignoreUnmonitored": true, }, }, diff --git a/packages/framework/presence/src/test/eventing.spec.ts b/packages/framework/presence/src/test/eventing.spec.ts index 104dd24d2090..77b18ab6b2ed 100644 --- a/packages/framework/presence/src/test/eventing.spec.ts +++ b/packages/framework/presence/src/test/eventing.spec.ts @@ -10,6 +10,7 @@ import type { SinonFakeTimers, SinonSpy } from "sinon"; import { useFakeTimers, spy } from "sinon"; import type { Attendee, WorkspaceAddress } from "../index.js"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; import { @@ -35,7 +36,7 @@ const attendeeUpdate = { "client1": { "rev": 0, "timestamp": 0, - "value": attendeeId1, + "value": serializableToOpaqueJson(attendeeId1), }, }, } as const; diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index 4d92ba58772a..7d2f557771f1 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -10,6 +10,7 @@ import { useFakeTimers, type SinonFakeTimers } from "sinon"; import type { Attendee, NotificationsManager, NotificationsWorkspace } from "../index.js"; import { Notifications } from "../index.js"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -127,7 +128,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId2), + }, }, }, "n:name:testNotificationWorkspace": { @@ -135,7 +140,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -180,7 +185,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, + [connectionId2]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId2), + }, }, }, "n:name:testNotificationWorkspace": { @@ -188,7 +197,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -259,7 +268,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, + [connectionId3]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId3), + }, }, }, "n:name:testNotificationWorkspace": { @@ -267,7 +280,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -337,7 +350,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, + [connectionId3]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId3), + }, }, }, "n:name:testNotificationWorkspace": { @@ -345,7 +362,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "oldId", "args": [41] }, + "value": serializableToOpaqueJson({ "name": "oldId", "args": [41] }), "ignoreUnmonitored": true, }, }, @@ -403,7 +420,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, + [connectionId3]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId3), + }, }, }, "n:name:testNotificationWorkspace": { @@ -411,7 +432,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [43] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [43] }), "ignoreUnmonitored": true, }, }, @@ -475,7 +496,11 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, + [connectionId3]: { + "rev": 0, + "timestamp": 1000, + "value": serializableToOpaqueJson(attendeeId3), + }, }, }, "n:name:testNotificationWorkspace": { @@ -483,7 +508,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [44] }, + "value": serializableToOpaqueJson({ "name": "newId", "args": [44] }), "ignoreUnmonitored": true, }, }, diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index 5627f9fd39cc..f72921b6e832 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -9,6 +9,7 @@ import { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/internal import type { SinonFakeTimers } from "sinon"; import { useFakeTimers, spy } from "sinon"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import type { AttendeeId } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; import type { SystemWorkspaceDatastore } from "../systemWorkspace.js"; @@ -28,7 +29,7 @@ const attendee4SystemWorkspaceDatastore = { ["client4" as AttendeeId]: { "rev": 0, "timestamp": 700, - "value": createSpecificAttendeeId("attendeeId-4"), + "value": serializableToOpaqueJson(createSpecificAttendeeId("attendeeId-4")), }, }, } as const satisfies SystemWorkspaceDatastore; @@ -105,7 +106,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": initialTime, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -182,7 +183,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": initialTime, - "value": attendeeId2, + "value": serializableToOpaqueJson(attendeeId2), }, }, }, @@ -210,7 +211,7 @@ describe("Presence", () => { "client1": { "rev": 0, "timestamp": 0, - "value": attendeeId1, + "value": serializableToOpaqueJson(attendeeId1), }, }, }; @@ -220,7 +221,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": {}, + "value": serializableToOpaqueJson({}), }, }, }; @@ -230,7 +231,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 0, "timestamp": 0, - "value": {}, + "value": serializableToOpaqueJson({}), "ignoreUnmonitored": true, }, }, @@ -328,7 +329,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": { x: 1, y: 1, z: 1 }, + "value": serializableToOpaqueJson({ x: 1, y: 1, z: 1 }), }, }, }, diff --git a/packages/framework/presence/src/test/presenceManager.spec.ts b/packages/framework/presence/src/test/presenceManager.spec.ts index 9dbbd945dd83..d1c8e14b3644 100644 --- a/packages/framework/presence/src/test/presenceManager.spec.ts +++ b/packages/framework/presence/src/test/presenceManager.spec.ts @@ -10,6 +10,7 @@ import type { SinonFakeTimers } from "sinon"; import { useFakeTimers } from "sinon"; import type { ClientConnectionId } from "../baseTypes.js"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import { AttendeeStatus, type Attendee } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; @@ -269,7 +270,7 @@ describe("Presence", () => { [collateralAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: collateralSessionId, + value: serializableToOpaqueJson(collateralSessionId), }, }, }); @@ -307,7 +308,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: collateralSessionId, + value: serializableToOpaqueJson(collateralSessionId), }, }, }); @@ -324,7 +325,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: collateralSessionId, + value: serializableToOpaqueJson(collateralSessionId), }, }, }); diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index ab2cbc5271fe..98480e75cadc 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -8,6 +8,7 @@ import type { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/int import { getUnexpectedLogErrorException } from "@fluidframework/test-utils/internal"; import type { SinonFakeTimers } from "sinon"; +import { serializableToOpaqueJson } from "../internalUtils.js"; import { createPresenceManager } from "../presenceManager.js"; import type { InboundClientJoinMessage, OutboundClientJoinMessage } from "../protocol.js"; import type { SystemWorkspaceDatastore } from "../systemWorkspace.js"; @@ -102,7 +103,7 @@ export function generateBasicClientJoin( [clientConnectionId]: { "rev": connectionOrder, "timestamp": fixedTime, - "value": attendeeId as AttendeeId, + "value": serializableToOpaqueJson(attendeeId as AttendeeId), }, }, }, From 90bf2a2b542ead98ddceb58da612f66adca58667 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Mon, 2 Jun 2025 17:05:17 -0700 Subject: [PATCH 04/13] docs(client-presence): update API reports after ignoring remaining type issue in presenceDatastoreManager.ts `PresenceDatastore` does not satisfy `DatastoreMessageContent` --- .../presence/api-report/presence.alpha.api.md | 34 +++++++++---------- .../presence/api-report/presence.beta.api.md | 22 ++++++------ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/framework/presence/api-report/presence.alpha.api.md b/packages/framework/presence/api-report/presence.alpha.api.md index 2ed0ce115573..0d6ae8a0fe71 100644 --- a/packages/framework/presence/api-report/presence.alpha.api.md +++ b/packages/framework/presence/api-report/presence.alpha.api.md @@ -86,7 +86,7 @@ export namespace InternalTypes { // @system (undocumented) export interface NotificationType { // (undocumented) - args: (JsonSerializable & JsonDeserialized)[]; + args: unknown[]; // (undocumented) name: string; } @@ -112,12 +112,12 @@ export namespace InternalTypes { // @system (undocumented) export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } // @system (undocumented) export interface ValueRequiredState extends ValueStateMetadata { // (undocumented) - value: JsonDeserialized; + value: OpaqueJsonDeserialized; } // @system (undocumented) export interface ValueStateMetadata { @@ -131,23 +131,23 @@ export namespace InternalTypes { // @alpha @system export namespace InternalUtilityTypes { // @system - export type IsNotificationListener = Event extends (...args: infer P) => void ? InternalUtilityTypes_2.IfSameType & JsonDeserialized

, true, false> : false; + export type IfNotificationListener = Event extends (...args: infer P) => void ? InternalUtilityTypes_2.IfSameType, IfListener, Else> : Else; // @system - export type JsonDeserializedParameters any> = T extends (...args: infer P) => any ? JsonDeserialized

: never; + export type JsonDeserializedParameters any> = T extends (...args: infer P) => any ? JsonDeserialized

: never; // @system - export type JsonSerializableParameters any> = T extends (...args: infer P) => any ? JsonSerializable

: never; + export type JsonSerializableParameters any> = T extends (...args: infer P) => any ? JsonSerializable

: never; // @system export type NotificationListeners = { - [P in string & keyof E as IsNotificationListener extends true ? P : never]: E[P]; + [P in keyof E as IfNotificationListener]: E[P]; }; } // @beta export function latest(args: LatestArguments): InternalTypes.ManagerFactory, LatestRaw>; -// @beta +// @beta @input export interface LatestArguments { - local: JsonSerializable & JsonDeserialized & (object | null); + local: JsonSerializable; settings?: BroadcastControlSettings | undefined; } @@ -165,10 +165,10 @@ export interface LatestData { // @beta export function latestMap(args?: LatestMapArguments): InternalTypes.ManagerFactory, LatestMapRaw>; -// @beta +// @beta @input export interface LatestMapArguments { local?: { - [K in Keys]: JsonSerializable & JsonDeserialized; + [K in Keys]: JsonSerializable; }; settings?: BroadcastControlSettings | undefined; } @@ -210,7 +210,7 @@ export interface LatestMapRawEvents { }) => void; // @eventProperty localItemUpdated: (updatedItem: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; key: K; }) => void; // @eventProperty @@ -235,7 +235,7 @@ export interface LatestRaw { getRemotes(): IterableIterator>; getStateAttendees(): Attendee[]; get local(): DeepReadonly>; - set local(value: JsonSerializable & JsonDeserialized); + set local(value: JsonSerializable); readonly presence: Presence; } @@ -243,7 +243,7 @@ export interface LatestRaw { export interface LatestRawEvents { // @eventProperty localUpdated: (update: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; }) => void; // @eventProperty remoteUpdated: (update: LatestClientData) => void; @@ -251,8 +251,8 @@ export interface LatestRawEvents { // @alpha @sealed export interface NotificationEmitter> { - broadcast>(notificationName: K, ...args: Parameters): void; - unicast>(notificationName: K, targetAttendee: Attendee, ...args: Parameters): void; + broadcast>(notificationName: K, ...args: Parameters): void; + unicast>(notificationName: K, targetAttendee: Attendee, ...args: Parameters): void; } // @alpha @sealed @@ -339,7 +339,7 @@ export interface StateMap { get(key: K): DeepReadonly> | undefined; has(key: K): boolean; keys(): IterableIterator; - set(key: K, value: JsonSerializable & JsonDeserialized): this; + set(key: K, value: JsonSerializable): this; readonly size: number; } diff --git a/packages/framework/presence/api-report/presence.beta.api.md b/packages/framework/presence/api-report/presence.beta.api.md index 240e944302dd..9c8c964ea9e0 100644 --- a/packages/framework/presence/api-report/presence.beta.api.md +++ b/packages/framework/presence/api-report/presence.beta.api.md @@ -73,7 +73,7 @@ export namespace InternalTypes { // @system (undocumented) export interface NotificationType { // (undocumented) - args: (JsonSerializable & JsonDeserialized)[]; + args: unknown[]; // (undocumented) name: string; } @@ -99,12 +99,12 @@ export namespace InternalTypes { // @system (undocumented) export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } // @system (undocumented) export interface ValueRequiredState extends ValueStateMetadata { // (undocumented) - value: JsonDeserialized; + value: OpaqueJsonDeserialized; } // @system (undocumented) export interface ValueStateMetadata { @@ -118,9 +118,9 @@ export namespace InternalTypes { // @beta export function latest(args: LatestArguments): InternalTypes.ManagerFactory, LatestRaw>; -// @beta +// @beta @input export interface LatestArguments { - local: JsonSerializable & JsonDeserialized & (object | null); + local: JsonSerializable; settings?: BroadcastControlSettings | undefined; } @@ -138,10 +138,10 @@ export interface LatestData { // @beta export function latestMap(args?: LatestMapArguments): InternalTypes.ManagerFactory, LatestMapRaw>; -// @beta +// @beta @input export interface LatestMapArguments { local?: { - [K in Keys]: JsonSerializable & JsonDeserialized; + [K in Keys]: JsonSerializable; }; settings?: BroadcastControlSettings | undefined; } @@ -183,7 +183,7 @@ export interface LatestMapRawEvents { }) => void; // @eventProperty localItemUpdated: (updatedItem: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; key: K; }) => void; // @eventProperty @@ -208,7 +208,7 @@ export interface LatestRaw { getRemotes(): IterableIterator>; getStateAttendees(): Attendee[]; get local(): DeepReadonly>; - set local(value: JsonSerializable & JsonDeserialized); + set local(value: JsonSerializable); readonly presence: Presence; } @@ -216,7 +216,7 @@ export interface LatestRaw { export interface LatestRawEvents { // @eventProperty localUpdated: (update: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; }) => void; // @eventProperty remoteUpdated: (update: LatestClientData) => void; @@ -257,7 +257,7 @@ export interface StateMap { get(key: K): DeepReadonly> | undefined; has(key: K): boolean; keys(): IterableIterator; - set(key: K, value: JsonSerializable & JsonDeserialized): this; + set(key: K, value: JsonSerializable): this; readonly size: number; } From 24537dd69162fdf55547c82f32e148dba840fd35 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Tue, 3 Jun 2025 11:56:02 -0700 Subject: [PATCH 05/13] test(client-presence): correction after merge --- .../src/test/latestMapValueManager.spec.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/framework/presence/src/test/latestMapValueManager.spec.ts b/packages/framework/presence/src/test/latestMapValueManager.spec.ts index ab70e4fb44dd..a1601d8815cc 100644 --- a/packages/framework/presence/src/test/latestMapValueManager.spec.ts +++ b/packages/framework/presence/src/test/latestMapValueManager.spec.ts @@ -201,7 +201,7 @@ export function checkCompiles(): void { null: null, string: "string", number: 0, - true: true, + boolean: true, }, }), ); @@ -210,19 +210,20 @@ export function checkCompiles(): void { // map value types are not matched to specific key localPrimitiveMap.set("string", 1); - localPrimitiveMap.set("number", true); + // latestMap should infer that `true` or `false` is a valid value + // without use of `true as const` or explicit specification. + // That happened under PR #24752 unexpectedly. Presumably from some + // additional inference complication where `& JsonDeserialized` + // was used in `LatestMapArguments` that was relaxed in PR #247??. !!! <- to fill in + // Caller can always use explicit generic specification to be + // completely clear about the types. + localPrimitiveMap.set("number", false); // eslint-disable-next-line unicorn/no-null - localPrimitiveMap.set("true", null); + localPrimitiveMap.set("boolean", null); localPrimitiveMap.set("null", "null"); // @ts-expect-error with inferred keys only those named in init are accessible localPrimitiveMap.set("key3", "value"); // @ts-expect-error value of type value is not assignable localPrimitiveMap.set("null", { value: "value" }); - // latestMap infers that only `true` is a valid value without use of `false` or explicit specification. - // This happened under PR #24752 unexpectedly. Presumably from some additional inference complication. - // This is a better inferred result; so it can stand, but would not be terrible if the behavior changes. - // Caller can always use explicit generic specification to be completely clear about the types. - // @ts-expect-error Argument of type 'false' is not assignable to parameter of type 'string | number | true | null'. - localPrimitiveMap.set("true", false); } From 2aab49ee7d872035802fff304fc14bf884aac5da Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Tue, 3 Jun 2025 12:01:06 -0700 Subject: [PATCH 06/13] fix(client-presence): workaround "system:presence" incompatilities - `PresenceDatastore` -> `DatastoreMessageContent` - `InboundClientJoinMessage` -> `OutboundClientJoinMessage` --- .../presence/src/presenceDatastoreManager.ts | 28 +++++++++++++++++-- .../framework/presence/src/test/testUtils.ts | 27 ++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index bf1e4b696051..762f19f97ac9 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -172,12 +172,24 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (updateProviders.length > 3) { updateProviders.length = 3; } + // TODO: #??? investigate and address `PresenceDatastore` incompatibility with `DatastoreMessageContent` + // "system:presence" is always expected to be first in the data; so that it + // is processed first. Build copy without it to use spread and maintain order. + const datastoreWithoutSystem: Omit = { + ...this.datastore, + }; + delete datastoreWithoutSystem["system:presence"]; this.runtime.submitSignal({ type: joinMessageType, content: { sendTimestamp: Date.now(), avgLatency: this.averageLatency, - data: this.datastore, + data: { + "system:presence": + // this.datastore["system:presence"], // does not work directly + { ...this.datastore["system:presence"] }, + ...datastoreWithoutSystem, + }, updateProviders, }, }); @@ -331,13 +343,25 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } private broadcastAllKnownState(): void { + // TODO: #??? investigate and address `PresenceDatastore` incompatibility with `DatastoreMessageContent` + // "system:presence" is always expected to be first in the data; so that it + // is processed first. Build copy without it to use spread and maintain order. + const datastoreWithoutSystem: Omit = { + ...this.datastore, + }; + delete datastoreWithoutSystem["system:presence"]; this.runtime.submitSignal({ type: datastoreUpdateMessageType, content: { sendTimestamp: Date.now(), avgLatency: this.averageLatency, isComplete: true, - data: this.datastore, + data: { + "system:presence": + // this.datastore["system:presence"], // does not work directly + { ...this.datastore["system:presence"] }, + ...datastoreWithoutSystem, + }, }, }); this.refreshBroadcastRequested = false; diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 98480e75cadc..57792cd4855a 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -147,12 +147,35 @@ export function prepareConnectedPresence( quorumClientIds.length = 3; } - const expectedClientJoin: OutboundClientJoinMessage & - Partial> = generateBasicClientJoin(clock.now, { + // TODO: #??? investigate and address `InboundClientJoinMessage` incompatibility with `OutboundClientJoinMessage` + // The difference is likely from JsonSerializable expectation of + // OutboundClientJoinMessage as it applies to SystemDatastore whereas + // InboundClientJoinMessage has JsonDeserialized. + // "system:presence" is always expected to be first in the data; so that it + // is processed first. Build copy without it to use spread and maintain order. + // This is very similar to `PresenceDatastore` incompatibility with + // `DatastoreMessageContent` seen in `presenceDatastoreManager.ts`. + const inboundClientJoin = generateBasicClientJoin(clock.now, { attendeeId, clientConnectionId, updateProviders: quorumClientIds, }); + const inboundContentDataWithoutSystem: Omit< + InboundClientJoinMessage["content"]["data"], + "system:presence" + > = { ...inboundClientJoin.content.data }; + delete inboundContentDataWithoutSystem["system:presence"]; + const expectedClientJoin: OutboundClientJoinMessage & + Partial> = { + ...inboundClientJoin, + content: { + ...inboundClientJoin.content, + data: { + "system:presence": inboundClientJoin.content.data["system:presence"], + ...inboundContentDataWithoutSystem, + }, + }, + }; delete expectedClientJoin.clientId; runtime.signalsExpected.push([expectedClientJoin]); From 6d8059f1c0df9bda1d729f0a0c01d5fba864952e Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Tue, 3 Jun 2025 15:57:09 -0700 Subject: [PATCH 07/13] Revert "fix(client-presence): workaround "system:presence" incompatilities" This reverts commit 2aab49ee7d872035802fff304fc14bf884aac5da. --- .../presence/src/presenceDatastoreManager.ts | 28 ++----------------- .../framework/presence/src/test/testUtils.ts | 27 ++---------------- 2 files changed, 4 insertions(+), 51 deletions(-) diff --git a/packages/framework/presence/src/presenceDatastoreManager.ts b/packages/framework/presence/src/presenceDatastoreManager.ts index 762f19f97ac9..bf1e4b696051 100644 --- a/packages/framework/presence/src/presenceDatastoreManager.ts +++ b/packages/framework/presence/src/presenceDatastoreManager.ts @@ -172,24 +172,12 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { if (updateProviders.length > 3) { updateProviders.length = 3; } - // TODO: #??? investigate and address `PresenceDatastore` incompatibility with `DatastoreMessageContent` - // "system:presence" is always expected to be first in the data; so that it - // is processed first. Build copy without it to use spread and maintain order. - const datastoreWithoutSystem: Omit = { - ...this.datastore, - }; - delete datastoreWithoutSystem["system:presence"]; this.runtime.submitSignal({ type: joinMessageType, content: { sendTimestamp: Date.now(), avgLatency: this.averageLatency, - data: { - "system:presence": - // this.datastore["system:presence"], // does not work directly - { ...this.datastore["system:presence"] }, - ...datastoreWithoutSystem, - }, + data: this.datastore, updateProviders, }, }); @@ -343,25 +331,13 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager { } private broadcastAllKnownState(): void { - // TODO: #??? investigate and address `PresenceDatastore` incompatibility with `DatastoreMessageContent` - // "system:presence" is always expected to be first in the data; so that it - // is processed first. Build copy without it to use spread and maintain order. - const datastoreWithoutSystem: Omit = { - ...this.datastore, - }; - delete datastoreWithoutSystem["system:presence"]; this.runtime.submitSignal({ type: datastoreUpdateMessageType, content: { sendTimestamp: Date.now(), avgLatency: this.averageLatency, isComplete: true, - data: { - "system:presence": - // this.datastore["system:presence"], // does not work directly - { ...this.datastore["system:presence"] }, - ...datastoreWithoutSystem, - }, + data: this.datastore, }, }); this.refreshBroadcastRequested = false; diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 57792cd4855a..98480e75cadc 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -147,35 +147,12 @@ export function prepareConnectedPresence( quorumClientIds.length = 3; } - // TODO: #??? investigate and address `InboundClientJoinMessage` incompatibility with `OutboundClientJoinMessage` - // The difference is likely from JsonSerializable expectation of - // OutboundClientJoinMessage as it applies to SystemDatastore whereas - // InboundClientJoinMessage has JsonDeserialized. - // "system:presence" is always expected to be first in the data; so that it - // is processed first. Build copy without it to use spread and maintain order. - // This is very similar to `PresenceDatastore` incompatibility with - // `DatastoreMessageContent` seen in `presenceDatastoreManager.ts`. - const inboundClientJoin = generateBasicClientJoin(clock.now, { + const expectedClientJoin: OutboundClientJoinMessage & + Partial> = generateBasicClientJoin(clock.now, { attendeeId, clientConnectionId, updateProviders: quorumClientIds, }); - const inboundContentDataWithoutSystem: Omit< - InboundClientJoinMessage["content"]["data"], - "system:presence" - > = { ...inboundClientJoin.content.data }; - delete inboundContentDataWithoutSystem["system:presence"]; - const expectedClientJoin: OutboundClientJoinMessage & - Partial> = { - ...inboundClientJoin, - content: { - ...inboundClientJoin.content, - data: { - "system:presence": inboundClientJoin.content.data["system:presence"], - ...inboundContentDataWithoutSystem, - }, - }, - }; delete expectedClientJoin.clientId; runtime.signalsExpected.push([expectedClientJoin]); From 667aebce8840ff6ecb2859623cf9d8d003ea4d1f Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Tue, 3 Jun 2025 22:57:04 -0700 Subject: [PATCH 08/13] fix(client-presence): remove opacity from system datastore --- .../presence/api-report/presence.alpha.api.md | 4 +- .../presence/api-report/presence.beta.api.md | 4 +- .../presence/src/exposedInternalTypes.ts | 18 +++++++++ .../framework/presence/src/internalUtils.ts | 8 +++- .../framework/presence/src/systemWorkspace.ts | 37 ++++++++++++++----- .../presence/src/test/batching.spec.ts | 34 ++++++++--------- .../presence/src/test/eventing.spec.ts | 3 +- .../src/test/notificationsManager.spec.ts | 12 +++--- .../src/test/presenceDatastoreManager.spec.ts | 8 ++-- .../presence/src/test/presenceManager.spec.ts | 7 ++-- .../framework/presence/src/test/testUtils.ts | 3 +- 11 files changed, 88 insertions(+), 50 deletions(-) diff --git a/packages/framework/presence/api-report/presence.alpha.api.md b/packages/framework/presence/api-report/presence.alpha.api.md index 0d6ae8a0fe71..d4936c351d82 100644 --- a/packages/framework/presence/api-report/presence.alpha.api.md +++ b/packages/framework/presence/api-report/presence.alpha.api.md @@ -109,12 +109,12 @@ export namespace InternalTypes { } // @system (undocumented) export type ValueDirectoryOrState = ValueRequiredState | ValueDirectory; - // @system (undocumented) + // @system export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) value?: OpaqueJsonDeserialized; } - // @system (undocumented) + // @system export interface ValueRequiredState extends ValueStateMetadata { // (undocumented) value: OpaqueJsonDeserialized; diff --git a/packages/framework/presence/api-report/presence.beta.api.md b/packages/framework/presence/api-report/presence.beta.api.md index 9c8c964ea9e0..65c89257634d 100644 --- a/packages/framework/presence/api-report/presence.beta.api.md +++ b/packages/framework/presence/api-report/presence.beta.api.md @@ -96,12 +96,12 @@ export namespace InternalTypes { } // @system (undocumented) export type ValueDirectoryOrState = ValueRequiredState | ValueDirectory; - // @system (undocumented) + // @system export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) value?: OpaqueJsonDeserialized; } - // @system (undocumented) + // @system export interface ValueRequiredState extends ValueStateMetadata { // (undocumented) value: OpaqueJsonDeserialized; diff --git a/packages/framework/presence/src/exposedInternalTypes.ts b/packages/framework/presence/src/exposedInternalTypes.ts index f4df12c4b4be..4d2040d6d2d0 100644 --- a/packages/framework/presence/src/exposedInternalTypes.ts +++ b/packages/framework/presence/src/exposedInternalTypes.ts @@ -23,6 +23,12 @@ export namespace InternalTypes { } /** + * `ValueRequiredState` is used to represent a state that may have a value. + * And it includes standard metadata. + * + * @remarks + * See {@link InternalTypes.ValueRequiredState}. + * * @system */ export interface ValueOptionalState extends ValueStateMetadata { @@ -30,6 +36,18 @@ export namespace InternalTypes { } /** + * `ValueRequiredState` is used to represent a state that must have a value. + * And it includes standard metadata. + * + * @remarks + * The value is wrapped in `OpaqueJsonDeserialized` as uses are expected + * to involve generic or unknown types that will be filtered. It is here + * mostly as a convenience to the many such uses that would otherwise + * need to specify some wrapper themselves. + * + * For known cases, construct a custom interface that extends + * {@link InternalTypes.ValueStateMetadata}. + * * @system */ export interface ValueRequiredState extends ValueStateMetadata { diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index 27212b7dd32b..431877f73350 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -102,11 +102,15 @@ export function asDeeplyReadonlyDeserializedJson( return value as DeepReadonly> | undefined; } +type RevealOpaqueJsonDeserialized = T extends OpaqueJsonDeserialized + ? JsonDeserialized + : { [Key in keyof T]: RevealOpaqueJsonDeserialized }; + /** * Does nothing helper to reveal the Json type from a value's opaque Json type. */ -export function asDeserializedJson(value: OpaqueJsonDeserialized): JsonDeserialized { - return value as JsonDeserialized; +export function asDeserializedJson(value: T): RevealOpaqueJsonDeserialized { + return value as RevealOpaqueJsonDeserialized; } /** diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 5e814cab94e1..98e95e5a6675 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -10,19 +10,29 @@ import { assert } from "@fluidframework/core-utils/internal"; import type { ClientConnectionId } from "./baseTypes.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction } from "./internalTypes.js"; -import { asDeserializedJson, serializableToOpaqueJson } from "./internalUtils.js"; +import { asDeserializedJson } from "./internalUtils.js"; import type { Attendee, AttendeesEvents, AttendeeId, Presence } from "./presence.js"; import { AttendeeStatus } from "./presence.js"; import type { PresenceStatesInternal } from "./presenceStates.js"; import { TimerManager } from "./timerManager.js"; import type { AnyWorkspace, StatesWorkspaceSchema } from "./types.js"; +/** + * `ConnectionValueState` is known value state for `clientToSessionId` data. + * + * @remarks + * It is {@link InternalTypes.ValueRequiredState} with a known value type. + */ +interface ConnectionValueState extends InternalTypes.ValueStateMetadata { + value: AttendeeId; +} + /** * The system workspace's datastore structure. */ export interface SystemWorkspaceDatastore { clientToSessionId: { - [ConnectionId: ClientConnectionId]: InternalTypes.ValueRequiredState; + [ConnectionId: ClientConnectionId]: ConnectionValueState; }; } @@ -118,11 +128,21 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { public processUpdate( _received: number, _timeModifier: number, + /** + * Remote datastore typed to match {@link PresenceStatesInternal.processUpdate}'s + * `ValueUpdateRecord` type that uses {@link InternalTypes.ValueRequiredState} + * and expects and Opaque Json type. (We get away with a non-`unknown` value type + * per TypeScript's method parameter bivariance.) Proper type would be + * {@link ConnectionValueState} directly. + * {@link ClientConnectionId} use for index is also a deviation, but conveniently + * the accurate {@link AttendeeId} type is just a brand string, and + * {@link ClientConnectionId} is just `string`. + */ remoteDatastore: { clientToSessionId: { - [ConnectionId: ClientConnectionId]: InternalTypes.ValueRequiredState & { - ignoreUnmonitored?: true; - }; + [ConnectionId: ClientConnectionId]: InternalTypes.ValueRequiredState< + ConnectionValueState["value"] + >; }; }, senderConnectionId: ClientConnectionId, @@ -130,11 +150,10 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { const audienceMembers = this.audience.getMembers(); const postUpdateActions: PostUpdateAction[] = []; for (const [clientConnectionId, value] of Object.entries( - remoteDatastore.clientToSessionId, + asDeserializedJson(remoteDatastore.clientToSessionId), )) { - const attendeeId = asDeserializedJson(value.value); const { attendee, isJoining } = this.ensureAttendee( - attendeeId, + value.value, clientConnectionId, /* order */ value.rev, // If the attendee is present in audience OR if the attendee update is from the sending remote client itself, @@ -167,7 +186,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { this.datastore.clientToSessionId[clientConnectionId] = { rev: this.selfAttendee.order++, timestamp: Date.now(), - value: serializableToOpaqueJson(this.selfAttendee.attendeeId), + value: this.selfAttendee.attendeeId, }; // Mark 'Connected' remote attendees connections as stale diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index b1e2b155d894..fa6a54c87a28 100644 --- a/packages/framework/presence/src/test/batching.spec.ts +++ b/packages/framework/presence/src/test/batching.spec.ts @@ -70,7 +70,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -101,7 +101,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -132,7 +132,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -188,7 +188,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -236,7 +236,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -267,7 +267,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -344,7 +344,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -375,7 +375,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -450,7 +450,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -490,7 +490,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -564,7 +564,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -604,7 +604,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -672,7 +672,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -755,7 +755,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -785,7 +785,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -851,7 +851,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -895,7 +895,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, diff --git a/packages/framework/presence/src/test/eventing.spec.ts b/packages/framework/presence/src/test/eventing.spec.ts index 77b18ab6b2ed..104dd24d2090 100644 --- a/packages/framework/presence/src/test/eventing.spec.ts +++ b/packages/framework/presence/src/test/eventing.spec.ts @@ -10,7 +10,6 @@ import type { SinonFakeTimers, SinonSpy } from "sinon"; import { useFakeTimers, spy } from "sinon"; import type { Attendee, WorkspaceAddress } from "../index.js"; -import { serializableToOpaqueJson } from "../internalUtils.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; import { @@ -36,7 +35,7 @@ const attendeeUpdate = { "client1": { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson(attendeeId1), + "value": attendeeId1, }, }, } as const; diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index 7d2f557771f1..fd527bf87afa 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -131,7 +131,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -188,7 +188,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -271,7 +271,7 @@ describe("Presence", () => { [connectionId3]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId3), + "value": attendeeId3, }, }, }, @@ -353,7 +353,7 @@ describe("Presence", () => { [connectionId3]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId3), + "value": attendeeId3, }, }, }, @@ -423,7 +423,7 @@ describe("Presence", () => { [connectionId3]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId3), + "value": attendeeId3, }, }, }, @@ -499,7 +499,7 @@ describe("Presence", () => { [connectionId3]: { "rev": 0, "timestamp": 1000, - "value": serializableToOpaqueJson(attendeeId3), + "value": attendeeId3, }, }, }, diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index f72921b6e832..1fdc55bb2914 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -29,7 +29,7 @@ const attendee4SystemWorkspaceDatastore = { ["client4" as AttendeeId]: { "rev": 0, "timestamp": 700, - "value": serializableToOpaqueJson(createSpecificAttendeeId("attendeeId-4")), + "value": createSpecificAttendeeId("attendeeId-4"), }, }, } as const satisfies SystemWorkspaceDatastore; @@ -106,7 +106,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": initialTime, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -183,7 +183,7 @@ describe("Presence", () => { [connectionId2]: { "rev": 0, "timestamp": initialTime, - "value": serializableToOpaqueJson(attendeeId2), + "value": attendeeId2, }, }, }, @@ -211,7 +211,7 @@ describe("Presence", () => { "client1": { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson(attendeeId1), + "value": attendeeId1, }, }, }; diff --git a/packages/framework/presence/src/test/presenceManager.spec.ts b/packages/framework/presence/src/test/presenceManager.spec.ts index d1c8e14b3644..9dbbd945dd83 100644 --- a/packages/framework/presence/src/test/presenceManager.spec.ts +++ b/packages/framework/presence/src/test/presenceManager.spec.ts @@ -10,7 +10,6 @@ import type { SinonFakeTimers } from "sinon"; import { useFakeTimers } from "sinon"; import type { ClientConnectionId } from "../baseTypes.js"; -import { serializableToOpaqueJson } from "../internalUtils.js"; import { AttendeeStatus, type Attendee } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; @@ -270,7 +269,7 @@ describe("Presence", () => { [collateralAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: serializableToOpaqueJson(collateralSessionId), + value: collateralSessionId, }, }, }); @@ -308,7 +307,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: serializableToOpaqueJson(collateralSessionId), + value: collateralSessionId, }, }, }); @@ -325,7 +324,7 @@ describe("Presence", () => { [oldAttendeeConnectionId]: { rev: 0, timestamp: 0, - value: serializableToOpaqueJson(collateralSessionId), + value: collateralSessionId, }, }, }); diff --git a/packages/framework/presence/src/test/testUtils.ts b/packages/framework/presence/src/test/testUtils.ts index 98480e75cadc..ab2cbc5271fe 100644 --- a/packages/framework/presence/src/test/testUtils.ts +++ b/packages/framework/presence/src/test/testUtils.ts @@ -8,7 +8,6 @@ import type { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/int import { getUnexpectedLogErrorException } from "@fluidframework/test-utils/internal"; import type { SinonFakeTimers } from "sinon"; -import { serializableToOpaqueJson } from "../internalUtils.js"; import { createPresenceManager } from "../presenceManager.js"; import type { InboundClientJoinMessage, OutboundClientJoinMessage } from "../protocol.js"; import type { SystemWorkspaceDatastore } from "../systemWorkspace.js"; @@ -103,7 +102,7 @@ export function generateBasicClientJoin( [clientConnectionId]: { "rev": connectionOrder, "timestamp": fixedTime, - "value": serializableToOpaqueJson(attendeeId as AttendeeId), + "value": attendeeId as AttendeeId, }, }, }, From c6192992a39191b4cc035453a01d544ffc6f9d15 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Wed, 4 Jun 2025 13:17:52 -0700 Subject: [PATCH 09/13] docs(client-presence): comment additions --- .../framework/presence/src/internalUtils.ts | 20 +++++++++++++++---- .../framework/presence/src/systemWorkspace.ts | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index 431877f73350..b331cc0f019a 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -81,7 +81,7 @@ export function getOrCreateRecord(value: T): DeepReadonly { return value as DeepReadonly; @@ -94,7 +94,8 @@ export function asDeeplyReadonlyDeserializedJson( value: OpaqueJsonDeserialized | undefined, ): DeepReadonly> | undefined; /** - * Does nothing helper to apply deep immutability to a value's opaque Json type revealing the Json type. + * No-runtime-effect helper to apply deep immutability to a value's opaque JSON + * type, revealing the JSON type. */ export function asDeeplyReadonlyDeserializedJson( value: OpaqueJsonDeserialized | undefined, @@ -107,14 +108,25 @@ type RevealOpaqueJsonDeserialized = T extends OpaqueJsonDeserialized : { [Key in keyof T]: RevealOpaqueJsonDeserialized }; /** - * Does nothing helper to reveal the Json type from a value's opaque Json type. + * No-runtime-effect helper to reveal the JSON type from a value's opaque JSON + * types throughout a structure. + * + * @remarks + * {@link OpaqueJsonDeserialized} instances will be replaced shallowly such + * that nested instances are retained. */ export function asDeserializedJson(value: T): RevealOpaqueJsonDeserialized { return value as RevealOpaqueJsonDeserialized; } /** - * Does nothing helper to automatically cast Json type to Opaque Json type. + * No-runtime-effect helper to automatically cast JSON type to Opaque JSON type + * at outermost scope. + * + * @remarks + * Types that satisfy {@link JsonSerializable} may also be deserialized. Thus, + * the return type is both {@link OpaqueJsonSerializable} and + * {@link OpaqueJsonDeserialized}. */ export function serializableToOpaqueJson( value: JsonSerializable, diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 98e95e5a6675..8906ba1dbb13 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -131,7 +131,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { /** * Remote datastore typed to match {@link PresenceStatesInternal.processUpdate}'s * `ValueUpdateRecord` type that uses {@link InternalTypes.ValueRequiredState} - * and expects and Opaque Json type. (We get away with a non-`unknown` value type + * and expects and Opaque JSON type. (We get away with a non-`unknown` value type * per TypeScript's method parameter bivariance.) Proper type would be * {@link ConnectionValueState} directly. * {@link ClientConnectionId} use for index is also a deviation, but conveniently From cad419db1401df303e46cb232c57dfea049deed9 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Wed, 4 Jun 2025 13:28:14 -0700 Subject: [PATCH 10/13] refactor(client-presence): rename helpers - `serializableToOpaqueJson` -> `toOpaqueJson` - `asDeserializedJson` -> `revealOpaqueJson` --- .../framework/presence/src/internalUtils.ts | 4 +- .../presence/src/latestMapValueManager.ts | 6 +-- .../presence/src/latestValueManager.ts | 6 +-- .../presence/src/notificationsManager.ts | 8 ++-- .../framework/presence/src/systemWorkspace.ts | 4 +- .../presence/src/test/batching.spec.ts | 46 +++++++++---------- .../src/test/notificationsManager.spec.ts | 14 +++--- .../src/test/presenceDatastoreManager.spec.ts | 8 ++-- 8 files changed, 48 insertions(+), 48 deletions(-) diff --git a/packages/framework/presence/src/internalUtils.ts b/packages/framework/presence/src/internalUtils.ts index b331cc0f019a..3f2730f67298 100644 --- a/packages/framework/presence/src/internalUtils.ts +++ b/packages/framework/presence/src/internalUtils.ts @@ -115,7 +115,7 @@ type RevealOpaqueJsonDeserialized = T extends OpaqueJsonDeserialized * {@link OpaqueJsonDeserialized} instances will be replaced shallowly such * that nested instances are retained. */ -export function asDeserializedJson(value: T): RevealOpaqueJsonDeserialized { +export function revealOpaqueJson(value: T): RevealOpaqueJsonDeserialized { return value as RevealOpaqueJsonDeserialized; } @@ -128,7 +128,7 @@ export function asDeserializedJson(value: T): RevealOpaqueJsonDeserialized * the return type is both {@link OpaqueJsonSerializable} and * {@link OpaqueJsonDeserialized}. */ -export function serializableToOpaqueJson( +export function toOpaqueJson( value: JsonSerializable, ): OpaqueJsonSerializable & OpaqueJsonDeserialized { return value as OpaqueJsonSerializable & OpaqueJsonDeserialized; diff --git a/packages/framework/presence/src/latestMapValueManager.ts b/packages/framework/presence/src/latestMapValueManager.ts index ce493ac14306..51c8ab613426 100644 --- a/packages/framework/presence/src/latestMapValueManager.ts +++ b/packages/framework/presence/src/latestMapValueManager.ts @@ -21,7 +21,7 @@ import { asDeeplyReadonlyDeserializedJson, objectEntries, objectKeys, - serializableToOpaqueJson, + toOpaqueJson, } from "./internalUtils.js"; import type { LatestClientData, LatestData, LatestMetadata } from "./latestValueTypes.js"; import type { AttendeeId, Attendee, Presence, SpecificAttendee } from "./presence.js"; @@ -303,7 +303,7 @@ class ValueMapImpl implements StateMap { return this.value.items[key]?.value !== undefined; } public set(key: K, inValue: JsonSerializable): this { - const value = serializableToOpaqueJson(inValue); + const value = toOpaqueJson(inValue); if (!(key in this.value.items)) { this.countDefined += 1; this.value.items[key] = { rev: 0, timestamp: 0, value }; @@ -567,7 +567,7 @@ export function latestMap< value.items[key] = { rev: 0, timestamp, - value: serializableToOpaqueJson(initialValues[key]), + value: toOpaqueJson(initialValues[key]), }; } } diff --git a/packages/framework/presence/src/latestValueManager.ts b/packages/framework/presence/src/latestValueManager.ts index f94061adcaaf..a3fc9c04ad32 100644 --- a/packages/framework/presence/src/latestValueManager.ts +++ b/packages/framework/presence/src/latestValueManager.ts @@ -20,7 +20,7 @@ import { asDeeplyReadonly, asDeeplyReadonlyDeserializedJson, objectEntries, - serializableToOpaqueJson, + toOpaqueJson, } from "./internalUtils.js"; import type { LatestClientData, LatestData } from "./latestValueTypes.js"; import type { Attendee, Presence } from "./presence.js"; @@ -125,7 +125,7 @@ class LatestValueManagerImpl public set local(value: JsonSerializable) { this.value.rev += 1; this.value.timestamp = Date.now(); - this.value.value = serializableToOpaqueJson(value); + this.value.value = toOpaqueJson(value); this.datastore.localUpdate(this.key, this.value, { allowableUpdateLatencyMs: this.controls.allowableUpdateLatencyMs, }); @@ -229,7 +229,7 @@ export function latest( // Latest takes ownership of the initial local value but makes a shallow // copy for basic protection. - const opaqueLocal = serializableToOpaqueJson(local); + const opaqueLocal = toOpaqueJson(local); const value: InternalTypes.ValueRequiredState = { rev: 0, timestamp: Date.now(), diff --git a/packages/framework/presence/src/notificationsManager.ts b/packages/framework/presence/src/notificationsManager.ts index 0cff2ee1adb4..dfd6c9d8cda0 100644 --- a/packages/framework/presence/src/notificationsManager.ts +++ b/packages/framework/presence/src/notificationsManager.ts @@ -10,7 +10,7 @@ import type { JsonTypeWith } from "@fluidframework/core-interfaces/internal"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { InternalUtilityTypes } from "./exposedUtilityTypes.js"; import type { PostUpdateAction, ValueManager } from "./internalTypes.js"; -import { asDeserializedJson, serializableToOpaqueJson } from "./internalUtils.js"; +import { revealOpaqueJson, toOpaqueJson } from "./internalUtils.js"; import type { Attendee, PresenceWithNotifications as Presence } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; import { brandIVM } from "./valueManager.js"; @@ -178,7 +178,7 @@ class NotificationsManagerImpl< { rev: 0, timestamp: 0, - value: serializableToOpaqueJson({ + value: toOpaqueJson({ name, args: [...(args as JsonTypeWith[])], }), @@ -194,7 +194,7 @@ class NotificationsManagerImpl< { rev: 0, timestamp: 0, - value: serializableToOpaqueJson({ + value: toOpaqueJson({ name, args: [...(args as JsonTypeWith[])], }), @@ -244,7 +244,7 @@ class NotificationsManagerImpl< updateValue: InternalTypes.ValueRequiredState, ): PostUpdateAction[] { const postUpdateActions: PostUpdateAction[] = []; - const value = asDeserializedJson(updateValue.value); + const value = revealOpaqueJson(updateValue.value); const eventName = value.name as keyof Listeners>; if (this.notificationsInternal.hasListeners(eventName)) { // Without schema validation, we don't know that the args are the correct type. diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 8906ba1dbb13..4d5628e25f0a 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -10,7 +10,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import type { ClientConnectionId } from "./baseTypes.js"; import type { InternalTypes } from "./exposedInternalTypes.js"; import type { PostUpdateAction } from "./internalTypes.js"; -import { asDeserializedJson } from "./internalUtils.js"; +import { revealOpaqueJson } from "./internalUtils.js"; import type { Attendee, AttendeesEvents, AttendeeId, Presence } from "./presence.js"; import { AttendeeStatus } from "./presence.js"; import type { PresenceStatesInternal } from "./presenceStates.js"; @@ -150,7 +150,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { const audienceMembers = this.audience.getMembers(); const postUpdateActions: PostUpdateAction[] = []; for (const [clientConnectionId, value] of Object.entries( - asDeserializedJson(remoteDatastore.clientToSessionId), + revealOpaqueJson(remoteDatastore.clientToSessionId), )) { const { attendee, isJoining } = this.ensureAttendee( value.value, diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index fa6a54c87a28..b7a3b6c4ae3c 100644 --- a/packages/framework/presence/src/test/batching.spec.ts +++ b/packages/framework/presence/src/test/batching.spec.ts @@ -9,7 +9,7 @@ import { useFakeTimers, type SinonFakeTimers } from "sinon"; import type { NotificationsWorkspace } from "../index.js"; import { Notifications, StateFactory } from "../index.js"; -import { serializableToOpaqueJson } from "../internalUtils.js"; +import { toOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -79,7 +79,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 0, }), }, @@ -110,7 +110,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 42, }), }, @@ -141,7 +141,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1020, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 84, }), }, @@ -197,7 +197,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 0, }), }, @@ -245,7 +245,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1060, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 22, }), }, @@ -276,7 +276,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 6, "timestamp": 1140, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 90, }), }, @@ -353,7 +353,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 34, }), }, @@ -384,7 +384,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 5, "timestamp": 1220, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 90, }), }, @@ -459,7 +459,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 0, }), }, @@ -468,7 +468,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 0, }), }, @@ -499,7 +499,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 34, }), }, @@ -508,7 +508,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 1, "timestamp": 1110, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 56, }), }, @@ -573,7 +573,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 34, }), }, @@ -582,7 +582,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "message": "will be queued", }), }, @@ -613,7 +613,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": serializableToOpaqueJson({ "message": "final message" }), + "value": toOpaqueJson({ "message": "final message" }), }, }, }, @@ -681,7 +681,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 34, }), }, @@ -692,7 +692,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "message": "final message", }), }, @@ -764,7 +764,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [77] }), + "value": toOpaqueJson({ "name": "newId", "args": [77] }), "ignoreUnmonitored": true, }, }, @@ -794,7 +794,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [88] }), + "value": toOpaqueJson({ "name": "newId", "args": [88] }), "ignoreUnmonitored": true, }, }, @@ -860,7 +860,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1040, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "num": 56, }), }, @@ -871,7 +871,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "name": "newId", "args": [99], }), @@ -904,7 +904,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ + "value": toOpaqueJson({ "name": "newId", "args": [111], }), diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index ecdb3b4ed63a..3d4e4404b3bf 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -15,7 +15,7 @@ import type { NotificationsWorkspace, } from "../index.js"; import { Notifications } from "../index.js"; -import { serializableToOpaqueJson } from "../internalUtils.js"; +import { toOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -144,7 +144,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -201,7 +201,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -284,7 +284,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [42] }), + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -366,7 +366,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "oldId", "args": [41] }), + "value": toOpaqueJson({ "name": "oldId", "args": [41] }), "ignoreUnmonitored": true, }, }, @@ -436,7 +436,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [43] }), + "value": toOpaqueJson({ "name": "newId", "args": [43] }), "ignoreUnmonitored": true, }, }, @@ -512,7 +512,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({ "name": "newId", "args": [44] }), + "value": toOpaqueJson({ "name": "newId", "args": [44] }), "ignoreUnmonitored": true, }, }, diff --git a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts index c1f7e8239f7c..a8c03795ccb4 100644 --- a/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts +++ b/packages/framework/presence/src/test/presenceDatastoreManager.spec.ts @@ -9,7 +9,7 @@ import { EventAndErrorTrackingLogger } from "@fluidframework/test-utils/internal import type { SinonFakeTimers } from "sinon"; import { useFakeTimers, spy } from "sinon"; -import { serializableToOpaqueJson } from "../internalUtils.js"; +import { toOpaqueJson } from "../internalUtils.js"; import type { AttendeeId } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; import type { InternalWorkspaceAddress } from "../protocol.js"; @@ -222,7 +222,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": serializableToOpaqueJson({}), + "value": toOpaqueJson({}), }, }, }; @@ -232,7 +232,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 0, "timestamp": 0, - "value": serializableToOpaqueJson({}), + "value": toOpaqueJson({}), "ignoreUnmonitored": true, }, }, @@ -330,7 +330,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": serializableToOpaqueJson({ x: 1, y: 1, z: 1 }), + "value": toOpaqueJson({ x: 1, y: 1, z: 1 }), }, }, }, From b6da44a5594d94652ca4aeb06601bd88591005f4 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 5 Jun 2025 23:32:47 -0700 Subject: [PATCH 11/13] revert(client-presence): unneeded code changes --- .../framework/presence/src/systemWorkspace.ts | 3 +- .../presence/src/test/batching.spec.ts | 90 ++++--------------- .../src/test/notificationsManager.spec.ts | 36 ++------ 3 files changed, 23 insertions(+), 106 deletions(-) diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 4d5628e25f0a..36201a560727 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -152,8 +152,9 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { for (const [clientConnectionId, value] of Object.entries( revealOpaqueJson(remoteDatastore.clientToSessionId), )) { + const attendeeId = value.value; const { attendee, isJoining } = this.ensureAttendee( - value.value, + attendeeId, clientConnectionId, /* order */ value.rev, // If the attendee is present in audience OR if the attendee update is from the sending remote client itself, diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index b7a3b6c4ae3c..ce450824fcec 100644 --- a/packages/framework/presence/src/test/batching.spec.ts +++ b/packages/framework/presence/src/test/batching.spec.ts @@ -67,11 +67,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -98,11 +94,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -129,11 +121,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -233,11 +221,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -264,11 +248,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -341,11 +321,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -372,11 +348,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -447,11 +419,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -487,11 +455,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -561,11 +525,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -601,11 +561,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -752,11 +708,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "n:name:testNotificationWorkspace": { @@ -782,11 +734,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "n:name:testNotificationWorkspace": { @@ -848,11 +796,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "s:name:testStateWorkspace": { @@ -892,11 +836,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "n:name:testNotificationWorkspace": { diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index 3d4e4404b3bf..d0613a728faf 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -132,11 +132,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "n:name:testNotificationWorkspace": { @@ -189,11 +185,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId2]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId2, - }, + [connectionId2]: { "rev": 0, "timestamp": 1000, "value": attendeeId2 }, }, }, "n:name:testNotificationWorkspace": { @@ -272,11 +264,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId3, - }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { @@ -354,11 +342,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId3, - }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { @@ -424,11 +408,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId3, - }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { @@ -500,11 +480,7 @@ describe("Presence", () => { "data": { "system:presence": { "clientToSessionId": { - [connectionId3]: { - "rev": 0, - "timestamp": 1000, - "value": attendeeId3, - }, + [connectionId3]: { "rev": 0, "timestamp": 1000, "value": attendeeId3 }, }, }, "n:name:testNotificationWorkspace": { From 8d3150bbdabdb37dfb5c993cd80f7acd6fc3a881 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Sat, 7 Jun 2025 14:39:35 -0700 Subject: [PATCH 12/13] docs(client-presence): typo fixes --- packages/framework/presence/src/systemWorkspace.ts | 4 ++-- .../framework/presence/src/test/notificationsManager.spec.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/framework/presence/src/systemWorkspace.ts b/packages/framework/presence/src/systemWorkspace.ts index 36201a560727..9d7a8907ecb2 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -131,11 +131,11 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { /** * Remote datastore typed to match {@link PresenceStatesInternal.processUpdate}'s * `ValueUpdateRecord` type that uses {@link InternalTypes.ValueRequiredState} - * and expects and Opaque JSON type. (We get away with a non-`unknown` value type + * and expects an Opaque JSON type. (We get away with a non-`unknown` value type * per TypeScript's method parameter bivariance.) Proper type would be * {@link ConnectionValueState} directly. * {@link ClientConnectionId} use for index is also a deviation, but conveniently - * the accurate {@link AttendeeId} type is just a brand string, and + * the accurate {@link AttendeeId} type is just a branded string, and * {@link ClientConnectionId} is just `string`. */ remoteDatastore: { diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index d0613a728faf..8dfe971b0afb 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -85,7 +85,8 @@ describe("Presence", () => { notificationsWorkspace.add( "testEvents", Notifications< - // Below explicit generic specification should not be required. + // Below explicit generic specification should not be required + // when default handler is specified. { newId: (id: number) => void; }, From d326c103f88900accd3f2627ebfe7c803a7e4057 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Mon, 9 Jun 2025 11:39:41 -0700 Subject: [PATCH 13/13] docs(client-presence): comment improvements --- packages/framework/presence/src/exposedInternalTypes.ts | 4 ++-- packages/framework/presence/src/exposedUtilityTypes.ts | 2 +- packages/framework/presence/src/latestValueManager.ts | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/framework/presence/src/exposedInternalTypes.ts b/packages/framework/presence/src/exposedInternalTypes.ts index 4d2040d6d2d0..ff746b10cbe9 100644 --- a/packages/framework/presence/src/exposedInternalTypes.ts +++ b/packages/framework/presence/src/exposedInternalTypes.ts @@ -23,7 +23,7 @@ export namespace InternalTypes { } /** - * `ValueRequiredState` is used to represent a state that may have a value. + * Represents a state that may have a value. * And it includes standard metadata. * * @remarks @@ -36,7 +36,7 @@ export namespace InternalTypes { } /** - * `ValueRequiredState` is used to represent a state that must have a value. + * Represents a state that must have a value. * And it includes standard metadata. * * @remarks diff --git a/packages/framework/presence/src/exposedUtilityTypes.ts b/packages/framework/presence/src/exposedUtilityTypes.ts index 70b4e9fe7049..dec9994efa2f 100644 --- a/packages/framework/presence/src/exposedUtilityTypes.ts +++ b/packages/framework/presence/src/exposedUtilityTypes.ts @@ -19,7 +19,7 @@ import type { // eslint-disable-next-line @typescript-eslint/no-namespace export namespace InternalUtilityTypes { /** - * `IfListener` iff the given type is an acceptable shape for a notification. + * Yields `IfListener` when the given type is an acceptable shape for a notification. * `Else` otherwise. * * @system diff --git a/packages/framework/presence/src/latestValueManager.ts b/packages/framework/presence/src/latestValueManager.ts index a3fc9c04ad32..bed1839d8cdb 100644 --- a/packages/framework/presence/src/latestValueManager.ts +++ b/packages/framework/presence/src/latestValueManager.ts @@ -193,7 +193,6 @@ class LatestValueManagerImpl * * @param value - The object to clone * @returns A shallow clone of the input value - * @internal */ export function shallowCloneNullableObject(value: T): T { return value === null ? value : shallowCloneObject(value); @@ -208,6 +207,10 @@ export function shallowCloneNullableObject(value: T): T export interface LatestArguments { /** * The initial value of the local state. + * + * @remarks + * `latest` assumes ownership of the value and its references. + * Make a deep clone before passing, if needed. */ local: JsonSerializable;