diff --git a/packages/framework/presence/api-report/presence.alpha.api.md b/packages/framework/presence/api-report/presence.alpha.api.md index 2ed0ce115573..d4936c351d82 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; } @@ -109,15 +109,15 @@ export namespace InternalTypes { } // @system (undocumented) export type ValueDirectoryOrState = ValueRequiredState | ValueDirectory; - // @system (undocumented) + // @system export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } - // @system (undocumented) + // @system 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..65c89257634d 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; } @@ -96,15 +96,15 @@ export namespace InternalTypes { } // @system (undocumented) export type ValueDirectoryOrState = ValueRequiredState | ValueDirectory; - // @system (undocumented) + // @system export interface ValueOptionalState extends ValueStateMetadata { // (undocumented) - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } - // @system (undocumented) + // @system 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; } diff --git a/packages/framework/presence/src/exposedInternalTypes.ts b/packages/framework/presence/src/exposedInternalTypes.ts index 46e457098129..ff746b10cbe9 100644 --- a/packages/framework/presence/src/exposedInternalTypes.ts +++ b/packages/framework/presence/src/exposedInternalTypes.ts @@ -3,10 +3,7 @@ * Licensed under the MIT License. */ -import type { - JsonDeserialized, - JsonSerializable, -} 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 @@ -26,17 +23,35 @@ export namespace InternalTypes { } /** + * Represents a state that may have a value. + * And it includes standard metadata. + * + * @remarks + * See {@link InternalTypes.ValueRequiredState}. + * * @system */ export interface ValueOptionalState extends ValueStateMetadata { - value?: JsonDeserialized; + value?: OpaqueJsonDeserialized; } /** + * Represents 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 { - value: JsonDeserialized; + value: OpaqueJsonDeserialized; } /** @@ -121,6 +136,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 a6df51d9f084..dec9994efa2f 100644 --- a/packages/framework/presence/src/exposedUtilityTypes.ts +++ b/packages/framework/presence/src/exposedUtilityTypes.ts @@ -19,18 +19,16 @@ 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. + * Yields `IfListener` when the given type is an acceptable shape for a notification. + * `Else` otherwise. * * @system */ - export type IsNotificationListener = Event extends (...args: infer P) => void - ? CoreInternalUtilityTypes.IfSameType< - P, - JsonSerializable

& JsonDeserialized

, - true, - false - > - : false; + export type IfNotificationListener = Event extends ( + ...args: infer P + ) => void + ? CoreInternalUtilityTypes.IfSameType, IfListener, Else> + : Else; /** * Used to specify the kinds of notifications emitted by a {@link NotificationListenable}. @@ -52,7 +50,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 +58,7 @@ export namespace InternalUtilityTypes { * * @system */ - export type JsonDeserializedParameters any> = T extends ( + export type JsonDeserializedParameters any> = T extends ( ...args: infer P ) => any ? JsonDeserialized

@@ -71,7 +69,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..3f2730f67298 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,55 @@ export function getOrCreateRecord(value: T): DeepReadonly { return value as DeepReadonly; } + +export function asDeeplyReadonlyDeserializedJson( + value: OpaqueJsonDeserialized, +): DeepReadonly>; +export function asDeeplyReadonlyDeserializedJson( + value: OpaqueJsonDeserialized | undefined, +): DeepReadonly> | undefined; +/** + * 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, +): DeepReadonly> | undefined { + return value as DeepReadonly> | undefined; +} + +type RevealOpaqueJsonDeserialized = T extends OpaqueJsonDeserialized + ? JsonDeserialized + : { [Key in keyof T]: RevealOpaqueJsonDeserialized }; + +/** + * 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 revealOpaqueJson(value: T): RevealOpaqueJsonDeserialized { + return value as RevealOpaqueJsonDeserialized; +} + +/** + * 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 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 44e1adff0f22..51c8ab613426 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, + asDeeplyReadonlyDeserializedJson, + objectEntries, + objectKeys, + toOpaqueJson, +} 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"; @@ -122,7 +128,7 @@ export interface LatestMapRawEvents { * @eventProperty */ localItemUpdated: (updatedItem: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; key: K; }) => void; @@ -196,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. @@ -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): this { + const value = toOpaqueJson(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 { @@ -519,7 +527,7 @@ export interface LatestMapArguments & JsonDeserialized; + [K in Keys]: JsonSerializable; }; /** @@ -559,7 +567,7 @@ export function latestMap< value.items[key] = { rev: 0, timestamp, - value: initialValues[key], + value: toOpaqueJson(initialValues[key]), }; } } diff --git a/packages/framework/presence/src/latestValueManager.ts b/packages/framework/presence/src/latestValueManager.ts index 08d0fc16732c..bed1839d8cdb 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, + objectEntries, + toOpaqueJson, +} from "./internalUtils.js"; import type { LatestClientData, LatestData } from "./latestValueTypes.js"; import type { Attendee, Presence } from "./presence.js"; import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js"; @@ -42,7 +47,7 @@ export interface LatestRawEvents { * @eventProperty */ localUpdated: (update: { - value: DeepReadonly & JsonDeserialized>; + value: DeepReadonly>; }) => void; } @@ -78,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. @@ -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) { + public set local(value: JsonSerializable) { this.value.rev += 1; this.value.timestamp = Date.now(); - this.value.value = value; + this.value.value = toOpaqueJson(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,38 @@ 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 + */ +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. + * + * @remarks + * `latest` assumes ownership of the value and its references. + * Make a deep clone before passing, if needed. */ - // eslint-disable-next-line @rushstack/no-new-null - local: JsonSerializable & JsonDeserialized & (object | null); + local: JsonSerializable; /** * See {@link BroadcastControlSettings}. @@ -213,10 +232,11 @@ export function latest( // Latest takes ownership of the initial local value but makes a shallow // copy for basic protection. + const opaqueLocal = toOpaqueJson(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..dfd6c9d8cda0 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 { 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"; @@ -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: toOpaqueJson({ + 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: toOpaqueJson({ + 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 = 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. // 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..9d7a8907ecb2 100644 --- a/packages/framework/presence/src/systemWorkspace.ts +++ b/packages/framework/presence/src/systemWorkspace.ts @@ -10,18 +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 { revealOpaqueJson } 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; }; } @@ -117,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 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 branded string, and + * {@link ClientConnectionId} is just `string`. + */ remoteDatastore: { clientToSessionId: { - [ConnectionId: ClientConnectionId]: InternalTypes.ValueRequiredState & { - ignoreUnmonitored?: true; - }; + [ConnectionId: ClientConnectionId]: InternalTypes.ValueRequiredState< + ConnectionValueState["value"] + >; }; }, senderConnectionId: ClientConnectionId, @@ -129,7 +150,7 @@ class SystemWorkspaceImpl implements PresenceStatesInternal, SystemWorkspace { const audienceMembers = this.audience.getMembers(); const postUpdateActions: PostUpdateAction[] = []; for (const [clientConnectionId, value] of Object.entries( - remoteDatastore.clientToSessionId, + revealOpaqueJson(remoteDatastore.clientToSessionId), )) { const attendeeId = value.value; const { attendee, isJoining } = this.ensureAttendee( @@ -146,8 +167,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 { diff --git a/packages/framework/presence/src/test/batching.spec.ts b/packages/framework/presence/src/test/batching.spec.ts index fa74fefc29c5..ce450824fcec 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 { toOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -66,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": { @@ -78,9 +75,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": toOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -97,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": { @@ -109,9 +102,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": { + "value": toOpaqueJson({ "num": 42, - }, + }), }, }, }, @@ -128,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": { @@ -140,9 +129,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1020, - "value": { + "value": toOpaqueJson({ "num": 84, - }, + }), }, }, }, @@ -196,9 +185,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": toOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -232,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": { @@ -244,9 +229,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1060, - "value": { + "value": toOpaqueJson({ "num": 22, - }, + }), }, }, }, @@ -263,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": { @@ -275,9 +256,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 6, "timestamp": 1140, - "value": { + "value": toOpaqueJson({ "num": 90, - }, + }), }, }, }, @@ -340,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": { @@ -352,9 +329,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": { + "value": toOpaqueJson({ "num": 34, - }, + }), }, }, }, @@ -371,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": { @@ -383,9 +356,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 5, "timestamp": 1220, - "value": { + "value": toOpaqueJson({ "num": 90, - }, + }), }, }, }, @@ -446,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": { @@ -458,18 +427,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": toOpaqueJson({ "num": 0, - }, + }), }, }, "immediateUpdate": { [attendeeId2]: { "rev": 0, "timestamp": 1010, - "value": { + "value": toOpaqueJson({ "num": 0, - }, + }), }, }, }, @@ -486,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": { @@ -498,18 +463,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1100, - "value": { + "value": toOpaqueJson({ "num": 34, - }, + }), }, }, "immediateUpdate": { [attendeeId2]: { "rev": 1, "timestamp": 1110, - "value": { + "value": toOpaqueJson({ "num": 56, - }, + }), }, }, }, @@ -560,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": { @@ -572,18 +533,18 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": { + "value": toOpaqueJson({ "num": 34, - }, + }), }, }, "note": { [attendeeId2]: { "rev": 1, "timestamp": 1020, - "value": { + "value": toOpaqueJson({ "message": "will be queued", - }, + }), }, }, }, @@ -608,7 +569,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": { "message": "final message" }, + "value": toOpaqueJson({ "message": "final message" }), }, }, }, @@ -676,9 +637,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1050, - "value": { + "value": toOpaqueJson({ "num": 34, - }, + }), }, }, }, @@ -687,9 +648,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 2, "timestamp": 1060, - "value": { + "value": toOpaqueJson({ "message": "final message", - }, + }), }, }, }, @@ -755,7 +716,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [77] }, + "value": toOpaqueJson({ "name": "newId", "args": [77] }), "ignoreUnmonitored": true, }, }, @@ -781,7 +742,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [88] }, + "value": toOpaqueJson({ "name": "newId", "args": [88] }), "ignoreUnmonitored": true, }, }, @@ -835,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": { @@ -847,9 +804,9 @@ describe("Presence", () => { [attendeeId2]: { "rev": 3, "timestamp": 1040, - "value": { + "value": toOpaqueJson({ "num": 56, - }, + }), }, }, }, @@ -858,10 +815,10 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { + "value": toOpaqueJson({ "name": "newId", "args": [99], - }, + }), "ignoreUnmonitored": true, }, }, @@ -879,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": { @@ -891,10 +844,10 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { + "value": toOpaqueJson({ "name": "newId", "args": [111], - }, + }), "ignoreUnmonitored": true, }, }, 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); } 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 } }), diff --git a/packages/framework/presence/src/test/notificationsManager.spec.ts b/packages/framework/presence/src/test/notificationsManager.spec.ts index c7d3555888c5..8dfe971b0afb 100644 --- a/packages/framework/presence/src/test/notificationsManager.spec.ts +++ b/packages/framework/presence/src/test/notificationsManager.spec.ts @@ -15,6 +15,7 @@ import type { NotificationsWorkspace, } from "../index.js"; import { Notifications } from "../index.js"; +import { toOpaqueJson } from "../internalUtils.js"; import type { createPresenceManager } from "../presenceManager.js"; import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js"; @@ -84,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; }, @@ -139,7 +141,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -192,7 +194,7 @@ describe("Presence", () => { [attendeeId2]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -271,7 +273,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [42] }, + "value": toOpaqueJson({ "name": "newId", "args": [42] }), "ignoreUnmonitored": true, }, }, @@ -349,7 +351,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "oldId", "args": [41] }, + "value": toOpaqueJson({ "name": "oldId", "args": [41] }), "ignoreUnmonitored": true, }, }, @@ -415,7 +417,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "name": "newId", "args": [43] }, + "value": toOpaqueJson({ "name": "newId", "args": [43] }), "ignoreUnmonitored": true, }, }, @@ -487,7 +489,7 @@ describe("Presence", () => { [attendeeId3]: { "rev": 0, "timestamp": 0, - "value": { "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 aa8e9ff051d9..a8c03795ccb4 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 { toOpaqueJson } from "../internalUtils.js"; import type { AttendeeId } from "../presence.js"; import { createPresenceManager } from "../presenceManager.js"; import type { InternalWorkspaceAddress } from "../protocol.js"; @@ -221,7 +222,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": {}, + "value": toOpaqueJson({}), }, }, }; @@ -231,7 +232,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 0, "timestamp": 0, - "value": {}, + "value": toOpaqueJson({}), "ignoreUnmonitored": true, }, }, @@ -329,7 +330,7 @@ describe("Presence", () => { [attendeeId1]: { "rev": 1, "timestamp": 0, - "value": { x: 1, y: 1, z: 1 }, + "value": toOpaqueJson({ x: 1, y: 1, z: 1 }), }, }, }, 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<