refactor(client-presence): stop fighting noUncheckedIndexAccess (#23592)

1. tsc noUncheckedIndexAccess option does not respect `in` checks, but
is safer overall. In those cases, use the more relaxed read and check
for `undefined` result (even though `undefined` is not a possible
value).
- In several places, code blocks are reordered per
unicorn/no-negated-condition.

2. Also assignment to a key with subsequent access is not recognized as
that key having a value. So, capture the value for subsequent use.

3. Use `!` where type `keyof T` is insufficient as well.

packages/framework/presence/src/latestMapValueManager.ts

@@ -372,10 +372,10 @@ class LatestMapValueManagerImpl<
 	public clientValue(client: ISessionClient): ReadonlyMap<Keys, LatestValueData<T>> {
 		const allKnownStates = this.datastore.knownValues(this.key);
 		const clientSessionId = client.sessionId;
-		if (!(clientSessionId in allKnownStates.states)) {
+		const clientStateMap = allKnownStates.states[clientSessionId];
+		if (clientStateMap === undefined) {
 			throw new Error("No entry for client");
 		}
-		const clientStateMap = allKnownStates.states[clientSessionId];
 		const items = new Map<Keys, LatestValueData<T>>();
 		for (const [key, item] of objectEntries(clientStateMap.items)) {
 			const value = item.value;
@@ -396,14 +396,12 @@ class LatestMapValueManagerImpl<
 	): void {
 		const allKnownStates = this.datastore.knownValues(this.key);
 		const clientSessionId: SpecificSessionClientId = client.sessionId;
-		if (!(clientSessionId in allKnownStates.states)) {
+		const currentState = (allKnownStates.states[clientSessionId] ??=
 			// New client - prepare new client state directory
-			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 objectEntries(value.items)) {

packages/framework/presence/src/latestValueManager.ts

@@ -130,12 +130,14 @@ class LatestValueManagerImpl<T, Key extends string>
 
 	public clientValue(client: ISessionClient): LatestValueData<T> {
 		const allKnownStates = this.datastore.knownValues(this.key);
-		const clientSessionId = client.sessionId;
-		if (clientSessionId in allKnownStates.states) {
-			const { value, rev: revision } = allKnownStates.states[clientSessionId];
-			return { value, metadata: { revision, timestamp: Date.now() } };
+		const clientState = allKnownStates.states[client.sessionId];
+		if (clientState === undefined) {
+			throw new Error("No entry for clientId");
 		}
-		throw new Error("No entry for clientId");
+		return {
+			value: clientState.value,
+			metadata: { revision: clientState.rev, timestamp: Date.now() },
+		};
 	}
 
 	public update(
@@ -145,11 +147,9 @@ class LatestValueManagerImpl<T, Key extends string>
 	): void {
 		const allKnownStates = this.datastore.knownValues(this.key);
 		const clientSessionId = client.sessionId;
-		if (clientSessionId in allKnownStates.states) {
-			const currentState = allKnownStates.states[clientSessionId];
-			if (currentState.rev >= value.rev) {
-				return;
-			}
+		const currentState = allKnownStates.states[clientSessionId];
+		if (currentState !== undefined && currentState.rev >= value.rev) {
+			return;
 		}
 		this.datastore.update(this.key, clientSessionId, value);
 		this.events.emit("updated", {

packages/framework/presence/src/presenceDatastoreManager.ts

@@ -115,11 +115,11 @@ function mergeGeneralDatastoreMessageContent(
 		const mergedData = queueDatastore[workspaceName] ?? {};
 
 		// Iterate over each value manager and its data, merging it as needed.
-		for (const valueManagerKey of Object.keys(workspaceData)) {
-			for (const [clientSessionId, value] of objectEntries(workspaceData[valueManagerKey])) {
-				mergedData[valueManagerKey] ??= {};
-				const oldData = mergedData[valueManagerKey][clientSessionId];
-				mergedData[valueManagerKey][clientSessionId] = mergeValueDirectory(
+		for (const [valueManagerKey, valueManagerValue] of objectEntries(workspaceData)) {
+			for (const [clientSessionId, value] of objectEntries(valueManagerValue)) {
+				const mergeObject = (mergedData[valueManagerKey] ??= {});
+				const oldData = mergeObject[clientSessionId];
+				mergeObject[clientSessionId] = mergeValueDirectory(
 					oldData,
 					value,
 					0, // local values do not need a time shift

packages/framework/presence/src/presenceStates.ts

@@ -280,8 +280,7 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
 				nodes[key as keyof TSchema] = newNodeData.manager;
 				if ("initialData" in newNodeData) {
 					const { value, allowableUpdateLatencyMs } = newNodeData.initialData;
-					datastore[key] ??= {};
-					datastore[key][clientSessionId] = value;
+					(datastore[key] ??= {})[clientSessionId] = value;
 					newValues[key] = value;
 					if (allowableUpdateLatencyMs !== undefined) {
 						cumulativeAllowableUpdateLatencyMs =
@@ -315,7 +314,9 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
 	} {
 		return {
 			self: this.runtime.clientSessionId,
-			states: this.datastore[key],
+			// Caller must only use `key`s that are part of `this.datastore`.
+			// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+			states: this.datastore[key]!,
 		};
 	}
 
@@ -364,13 +365,14 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
 		this.nodes[key] = nodeData.manager;
 		if ("initialData" in nodeData) {
 			const { value, allowableUpdateLatencyMs } = nodeData.initialData;
-			if (key in this.datastore) {
+			let datastoreValue = this.datastore[key];
+			if (datastoreValue === undefined) {
+				datastoreValue = this.datastore[key] = {};
+			} else {
 				// Already have received state from other clients. Kept in `all`.
 				// TODO: Send current `all` state to state manager.
-			} else {
-				this.datastore[key] = {};
 			}
-			this.datastore[key][this.runtime.clientSessionId] = value;
+			datastoreValue[this.runtime.clientSessionId] = value;
 			this.runtime.localUpdate(
 				{ [key]: value },
 				{
@@ -389,13 +391,14 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
 			this.controls.allowableUpdateLatencyMs = controls.allowableUpdateLatencyMs;
 		}
 		for (const [key, nodeFactory] of Object.entries(content)) {
-			if (key in this.nodes) {
-				const node = unbrandIVM(this.nodes[key]);
+			const brandedIVM = this.nodes[key];
+			if (brandedIVM === undefined) {
+				this.add(key, nodeFactory);
+			} else {
+				const node = unbrandIVM(brandedIVM);
 				if (!(node instanceof nodeFactory.instanceBase)) {
 					throw new TypeError(`State "${key}" previously created by different value manager.`);
 				}
-			} else {
-				this.add(key, nodeFactory);
 			}
 		}
 		return this as PresenceStates<TSchema & TSchemaAdditional>;
@@ -407,15 +410,16 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
 		remoteDatastore: ValueUpdateRecord,
 	): void {
 		for (const [key, remoteAllKnownState] of Object.entries(remoteDatastore)) {
-			if (key in this.nodes) {
-				const node = unbrandIVM(this.nodes[key]);
+			const brandedIVM = this.nodes[key];
+			if (brandedIVM === undefined) {
+				// Assume all broadcast state is meant to be kept even if not currently registered.
+				mergeUntrackedDatastore(key, remoteAllKnownState, this.datastore, timeModifier);
+			} else {
+				const node = unbrandIVM(brandedIVM);
 				for (const [clientSessionId, value] of objectEntries(remoteAllKnownState)) {
 					const client = this.runtime.lookupClient(clientSessionId);
 					node.update(client, received, value);
 				}
-			} else {
-				// Assume all broadcast state is meant to be kept even if not currently registered.
-				mergeUntrackedDatastore(key, remoteAllKnownState, this.datastore, timeModifier);
 			}
 		}
 	}

packages/framework/presence/tsconfig.json

@@ -7,7 +7,5 @@
 		"outDir": "./lib",
 		"noImplicitAny": true,
 		"noImplicitOverride": true,
-		"noUncheckedIndexedAccess": false,
-		"exactOptionalPropertyTypes": true,
 	},
 }