build(client-presence): type and lint fixes

Minor production runtime changes with introduction of getOrCreateRecord helper and retyping of Object.entries/keys utilities.

packages/framework/presence/.eslintrc.cjs

@@ -29,7 +29,6 @@ module.exports = {
 				],
 			},
 		],
-		"@fluid-internal/fluid/no-unchecked-record-access": "warn",
 	},
 	overrides: [
 		{

packages/framework/presence/api-report/presence.alpha.api.md

@@ -58,7 +58,7 @@ export function Latest<T extends object, Key extends string = string>(initialVal
 // @alpha
 export function LatestMap<T extends object, Keys extends string | number = string | number, RegistrationKey extends string = string>(initialValues?: {
     [K in Keys]: JsonSerializable<T> & JsonDeserialized<T>;
-}, controls?: BroadcastControlSettings): InternalTypes.ManagerFactory<RegistrationKey, InternalTypes.MapValueState<T>, LatestMapValueManager<T, Keys>>;
+}, controls?: BroadcastControlSettings): InternalTypes.ManagerFactory<RegistrationKey, InternalTypes.MapValueState<T, Keys>, LatestMapValueManager<T, Keys>>;
 
 // @alpha @sealed
 export interface LatestMapItemRemovedClientData<K extends string | number> {

packages/framework/presence/src/exposedInternalTypes.ts

@@ -60,13 +60,13 @@ export namespace InternalTypes {
 	/**
 	 * @system
 	 */
-	export interface MapValueState<T> {
+	export interface MapValueState<T, Keys extends string | number> {
 		rev: number;
 		items: {
 			// Caution: any particular item may or may not exist
 			// Typescript does not support absent keys without forcing type to also be undefined.
 			// See https://github.com/microsoft/TypeScript/issues/42810.
-			[name: string | number]: ValueOptionalState<T>;
+			[name in Keys]: ValueOptionalState<T>;
 		};
 	}
 

packages/framework/presence/src/internalTypes.ts

@@ -21,15 +21,6 @@ export interface ClientRecord<TValue extends InternalTypes.ValueDirectoryOrState
 	[ClientSessionId: ClientSessionId]: TValue;
 }
 
-/**
- * Object.entries retyped to support branded string-based keys.
- *
- * @internal
- */
-export const brandedObjectEntries = Object.entries as <K extends string, T>(
-	o: Record<K, T>,
-) => [K, T][];
-
 /**
  * This interface is a subset of (IContainerRuntime & IRuntimeInternal) and
  * (IFluidDataStoreRuntime) that is needed by the Presence States.

packages/framework/presence/src/internalUtils.ts

@@ -0,0 +1,72 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+/**
+ * Returns union of types of values in a record.
+ */
+export type RecordEntryTypes<T> = T[keyof T];
+
+type MapNumberIndicesToStrings<T> = {
+	[K in keyof T as K extends number ? `${K}` : K]: T[K];
+};
+
+type KeyValuePairs<T> = {
+	[K in keyof MapNumberIndicesToStrings<Required<T>>]: [K, Required<T>[K]];
+}[keyof MapNumberIndicesToStrings<Required<T>>][];
+
+type RequiredAndNotUndefined<T> = {
+	[K in keyof T]-?: Exclude<T[K], undefined>;
+};
+
+/**
+ * Object.entries retyped to preserve known keys and their types.
+ *
+ * @internal
+ */
+export const objectEntries = Object.entries as <T>(o: T) => KeyValuePairs<T>;
+
+/**
+ * Object.entries retyped to preserve known keys and their types.
+ *
+ * @remarks
+ * Given `T` should not contain `undefined` values. If it does, use
+ * {@link objectEntries} instead. Without `undefined` values, this
+ * typing provides best handling of objects with optional properties.
+ *
+ * @internal
+ */
+export const objectEntriesWithoutUndefined = Object.entries as <T>(
+	o: T,
+) => KeyValuePairs<RequiredAndNotUndefined<T>>;
+
+/**
+ * Object.keys retyped to preserve known keys and their types.
+ *
+ * @internal
+ */
+export const objectKeys = Object.keys as <T>(o: T) => (keyof MapNumberIndicesToStrings<T>)[];
+
+/**
+ * Retrieve a value from a record with the given key, or create a new entry if
+ * the key is not in the record.
+ *
+ * @param record - The record to index/update
+ * @param key - The key to lookup in the record
+ * @param defaultValue - a function which returns a default value. This is
+ * called and used to set an initial value for the given key in the record if
+ * none exists.
+ * @returns either the existing value for the given key, or the newly-created
+ * value (the result of `defaultValue`)
+ */
+export function getOrCreateRecord<K extends string | number | symbol, V>(
+	record: Record<K, V>,
+	key: K,
+	defaultValue: (key: K) => V,
+): V {
+	if (!(key in record)) {
+		record[key] = defaultValue(key);
+	}
+	return record[key];
+}

packages/framework/presence/src/latestMapValueManager.ts

@@ -9,6 +9,7 @@ import type { Listenable } from "@fluidframework/core-interfaces";
 import type { BroadcastControls, BroadcastControlSettings } from "./broadcastControls.js";
 import { OptionalBroadcastControl } from "./broadcastControls.js";
 import type { ValueManager } from "./internalTypes.js";
+import { objectEntries, objectKeys } from "./internalUtils.js";
 import type {
 	LatestValueClientData,
 	LatestValueData,
@@ -189,17 +190,28 @@ export interface ValueMap<K extends string | number, V> {
 class ValueMapImpl<T, K extends string | number> implements ValueMap<K, T> {
 	private countDefined: number;
 	public constructor(
-		private readonly value: InternalTypes.MapValueState<T>,
-		private readonly localUpdate: (updates: InternalTypes.MapValueState<T>) => void,
+		private readonly value: InternalTypes.MapValueState<T, K>,
+		private readonly localUpdate: (
+			updates: InternalTypes.MapValueState<
+				T,
+				// This should be `K`, but will only work if properties are optional.
+				string | number
+			>,
+		) => void,
 	) {
 		// All initial items are expected to be defined.
 		// TODO assert all defined and/or update type.
 		this.countDefined = Object.keys(value.items).length;
 	}
 
+	/**
+	 * Note: caller must ensure key exists in this.value.items.
+	 */
 	private updateItem(key: K, value: InternalTypes.ValueOptionalState<T>["value"]): void {
 		this.value.rev += 1;
-		const item = this.value.items[key];
+		// Caller is required to ensure key exists.
+		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+		const item = this.value.items[key]!;
 		item.rev += 1;
 		item.timestamp = Date.now();
 		if (value === undefined) {
@@ -231,10 +243,9 @@ class ValueMapImpl<T, K extends string | number> implements ValueMap<K, T> {
 		) => void,
 		thisArg?: unknown,
 	): void {
-		for (const [key, item] of Object.entries(this.value.items)) {
+		for (const [key, item] of objectEntries(this.value.items)) {
 			if (item.value !== undefined) {
-				// TODO: see about typing InternalTypes.MapValueState with K
-				callbackfn(item.value, key as K, this);
+				callbackfn(item.value, key, this);
 			}
 		}
 	}
@@ -257,9 +268,9 @@ class ValueMapImpl<T, K extends string | number> implements ValueMap<K, T> {
 	}
 	public keys(): IterableIterator<K> {
 		const keys: K[] = [];
-		for (const [key, item] of Object.entries(this.value.items)) {
+		for (const [key, item] of objectEntries(this.value.items)) {
 			if (item.value !== undefined) {
-				keys.push(key as K);
+				keys.push(key);
 			}
 		}
 		return keys[Symbol.iterator]();
@@ -312,7 +323,7 @@ class LatestMapValueManagerImpl<
 	Keys extends string | number = string | number,
 > implements
 		LatestMapValueManager<T, Keys>,
-		Required<ValueManager<T, InternalTypes.MapValueState<T>>>
+		Required<ValueManager<T, InternalTypes.MapValueState<T, Keys>>>
 {
 	public readonly events = createEmitter<LatestMapValueManagerEvents<T, Keys>>();
 	public readonly controls: OptionalBroadcastControl;
@@ -321,16 +332,16 @@ class LatestMapValueManagerImpl<
 		private readonly key: RegistrationKey,
 		private readonly datastore: StateDatastore<
 			RegistrationKey,
-			InternalTypes.MapValueState<T>
+			InternalTypes.MapValueState<T, Keys>
 		>,
-		public readonly value: InternalTypes.MapValueState<T>,
+		public readonly value: InternalTypes.MapValueState<T, Keys>,
 		controlSettings: BroadcastControlSettings | undefined,
 	) {
 		this.controls = new OptionalBroadcastControl(controlSettings);
 
 		this.local = new ValueMapImpl<T, Keys>(
 			value,
-			(updates: InternalTypes.MapValueState<T>) => {
+			(updates: InternalTypes.MapValueState<T, Keys>) => {
 				datastore.localUpdate(key, updates, {
 					allowableUpdateLatencyMs: this.controls.allowableUpdateLatencyMs,
 				});
@@ -342,7 +353,7 @@ class LatestMapValueManagerImpl<
 
 	public *clientValues(): IterableIterator<LatestMapValueClientData<T, Keys>> {
 		const allKnownStates = this.datastore.knownValues(this.key);
-		for (const clientSessionId of Object.keys(allKnownStates.states)) {
+		for (const clientSessionId of objectKeys(allKnownStates.states)) {
 			if (clientSessionId !== allKnownStates.self) {
 				const client = this.datastore.lookupClient(clientSessionId);
 				const items = this.clientValue(client);
@@ -353,7 +364,7 @@ class LatestMapValueManagerImpl<
 
 	public clients(): ISessionClient[] {
 		const allKnownStates = this.datastore.knownValues(this.key);
-		return Object.keys(allKnownStates.states)
+		return objectKeys(allKnownStates.states)
 			.filter((clientSessionId) => clientSessionId !== allKnownStates.self)
 			.map((clientSessionId) => this.datastore.lookupClient(clientSessionId));
 	}
@@ -366,10 +377,10 @@ class LatestMapValueManagerImpl<
 		}
 		const clientStateMap = allKnownStates.states[clientSessionId];
 		const items = new Map<Keys, LatestValueData<T>>();
-		for (const [key, item] of Object.entries(clientStateMap.items)) {
+		for (const [key, item] of objectEntries(clientStateMap.items)) {
 			const value = item.value;
 			if (value !== undefined) {
-				items.set(key as Keys, {
+				items.set(key, {
 					value,
 					metadata: { revision: item.rev, timestamp: item.timestamp },
 				});
@@ -381,20 +392,25 @@ class LatestMapValueManagerImpl<
 	public update<SpecificSessionClientId extends ClientSessionId>(
 		client: SpecificSessionClient<SpecificSessionClientId>,
 		_received: number,
-		value: InternalTypes.MapValueState<T>,
+		value: InternalTypes.MapValueState<T, string | number>,
 	): void {
 		const allKnownStates = this.datastore.knownValues(this.key);
 		const clientSessionId: SpecificSessionClientId = client.sessionId;
 		if (!(clientSessionId in allKnownStates.states)) {
 			// New client - prepare new client state directory
-			allKnownStates.states[clientSessionId] = { rev: value.rev, items: {} };
+			allKnownStates.states[clientSessionId] = {
+				rev: value.rev,
+				items: {} as unknown as InternalTypes.MapValueState<T, Keys>["items"],
+			};
 		}
 		const currentState = allKnownStates.states[clientSessionId];
 		// Accumulate individual update keys
 		const updatedItemKeys: Keys[] = [];
-		for (const [key, item] of Object.entries(value.items)) {
-			if (!(key in currentState.items) || currentState.items[key].rev < item.rev) {
-				updatedItemKeys.push(key as Keys);
+		for (const [key, item] of objectEntries(value.items)) {
+			// TODO: Key validation needs to be added here.
+			const validKey = key as Keys;
+			if (!(key in currentState.items) || currentState.items[validKey].rev < item.rev) {
+				updatedItemKeys.push(validKey);
 			}
 		}
 
@@ -411,7 +427,8 @@ class LatestMapValueManagerImpl<
 			items: new Map<Keys, LatestValueData<T>>(),
 		};
 		for (const key of updatedItemKeys) {
-			const item = value.items[key];
+			// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+			const item = value.items[key]!;
 			const hadPriorValue = currentState.items[key]?.value;
 			currentState.items[key] = item;
 			const metadata = { revision: item.rev, timestamp: item.timestamp };
@@ -452,22 +469,26 @@ export function LatestMap<
 	controls?: BroadcastControlSettings,
 ): InternalTypes.ManagerFactory<
 	RegistrationKey,
-	InternalTypes.MapValueState<T>,
+	InternalTypes.MapValueState<T, Keys>,
 	LatestMapValueManager<T, Keys>
 > {
 	const timestamp = Date.now();
-	const value: InternalTypes.MapValueState<T> = { rev: 0, items: {} };
+	const value: InternalTypes.MapValueState<
+		T,
+		// This should be `Keys`, but will only work if properties are optional.
+		string | number
+	> = { rev: 0, items: {} };
 	// LatestMapValueManager takes ownership of values within initialValues.
 	if (initialValues !== undefined) {
-		for (const key of Object.keys(initialValues)) {
-			value.items[key] = { rev: 0, timestamp, value: initialValues[key as Keys] };
+		for (const key of objectKeys(initialValues)) {
+			value.items[key] = { rev: 0, timestamp, value: initialValues[key] };
 		}
 	}
 	const factory = (
 		key: RegistrationKey,
 		datastoreHandle: InternalTypes.StateDatastoreHandle<
 			RegistrationKey,
-			InternalTypes.MapValueState<T>
+			InternalTypes.MapValueState<T, Keys>
 		>,
 	): {
 		initialData: { value: typeof value; allowableUpdateLatencyMs: number | undefined };
@@ -477,7 +498,7 @@ export function LatestMap<
 		manager: brandIVM<
 			LatestMapValueManagerImpl<T, RegistrationKey, Keys>,
 			T,
-			InternalTypes.MapValueState<T>
+			InternalTypes.MapValueState<T, Keys>
 		>(
 			new LatestMapValueManagerImpl(
 				key,

packages/framework/presence/src/latestValueManager.ts

@@ -9,7 +9,7 @@ import type { Listenable } from "@fluidframework/core-interfaces";
 import type { BroadcastControls, BroadcastControlSettings } from "./broadcastControls.js";
 import { OptionalBroadcastControl } from "./broadcastControls.js";
 import type { ValueManager } from "./internalTypes.js";
-import { brandedObjectEntries } from "./internalTypes.js";
+import { objectEntries } from "./internalUtils.js";
 import type { LatestValueClientData, LatestValueData } from "./latestValueTypes.js";
 import type { ISessionClient } from "./presence.js";
 import { datastoreFromHandle, type StateDatastore } from "./stateDatastore.js";
@@ -110,7 +110,7 @@ class LatestValueManagerImpl<T, Key extends string>
 
 	public *clientValues(): IterableIterator<LatestValueClientData<T>> {
 		const allKnownStates = this.datastore.knownValues(this.key);
-		for (const [clientSessionId, value] of brandedObjectEntries(allKnownStates.states)) {
+		for (const [clientSessionId, value] of objectEntries(allKnownStates.states)) {
 			if (clientSessionId !== allKnownStates.self) {
 				yield {
 					client: this.datastore.lookupClient(clientSessionId),

packages/framework/presence/src/presenceDatastoreManager.ts

@@ -9,8 +9,8 @@ import type { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/intern
 
 import type { ClientConnectionId } from "./baseTypes.js";
 import type { BroadcastControlSettings } from "./broadcastControls.js";
-import { brandedObjectEntries } from "./internalTypes.js";
 import type { IEphemeralRuntime } from "./internalTypes.js";
+import { objectEntries } from "./internalUtils.js";
 import type { ClientSessionId, ISessionClient } from "./presence.js";
 import type {
 	ClientUpdateEntry,
@@ -116,9 +116,7 @@ function mergeGeneralDatastoreMessageContent(
 
 		// Iterate over each value manager and its data, merging it as needed.
 		for (const valueManagerKey of Object.keys(workspaceData)) {
-			for (const [clientSessionId, value] of brandedObjectEntries(
-				workspaceData[valueManagerKey],
-			)) {
+			for (const [clientSessionId, value] of objectEntries(workspaceData[valueManagerKey])) {
 				mergedData[valueManagerKey] ??= {};
 				const oldData = mergedData[valueManagerKey][clientSessionId];
 				mergedData[valueManagerKey][clientSessionId] = mergeValueDirectory(
@@ -191,7 +189,8 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
 			return existing.internal.ensureContent(requestedContent, controls);
 		}
 
-		let workspaceDatastore = this.datastore[internalWorkspaceAddress];
+		let workspaceDatastore: ValueElementMap<PresenceStatesSchema> | undefined =
+			this.datastore[internalWorkspaceAddress];
 		if (workspaceDatastore === undefined) {
 			workspaceDatastore = this.datastore[internalWorkspaceAddress] = {};
 		}
@@ -302,7 +301,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
 		const clientConnectionId = this.runtime.clientId;
 		assert(clientConnectionId !== undefined, 0xa59 /* Client connected without clientId */);
 		const currentClientToSessionValueState =
-			this.datastore["system:presence"].clientToSessionId[clientConnectionId];
+			// When connected, `clientToSessionId` must always have current connection entry.
+			// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+			this.datastore["system:presence"].clientToSessionId[clientConnectionId]!;
 
 		const newMessage = {
 			sendTimestamp: Date.now(),