Skip to content

Commit 0052d4c

Browse files
authored
fix(client-presence): LatestValueManager with array comes across as object (#23662)
## Bug Description [ADO Bug 29110](https://dev.azure.com/fluidframework/internal/_workitems/edit/29110) Using an array (at least using `string[]`) as the type for `Latest` (`LatestValueManager`) appears to rehydrate as object rather than array. Example result: ```JSON { "0": "foo" } ``` Repro branch: https://github.com/jason-ha/FluidExamples/tree/presence_Latest_array Repro: connect two clients; make selection in one Expected: selection shows in other Actual: runtime error `indexOf` is not a member of ... ## Solution When we create a LatestValueManager with an`initialValue`, we make a shallow copy by doing `{ ..initialValue}`. So if `initialValue` is an array we're transforming it into an object. To avoid this we can check if it's an array first and maintain array. Was able to solve the repro with this fix.
1 parent 17372d0 commit 0052d4c

File tree

4 files changed

+65
-2
lines changed

4 files changed

+65
-2
lines changed

packages/common/core-utils/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export { Lazy, LazyPromise } from "./lazy.js";
1717
export type { PromiseCacheExpiry, PromiseCacheOptions } from "./promiseCache.js";
1818
export { PromiseCache } from "./promiseCache.js";
1919
export { Deferred } from "./promises.js";
20+
export { shallowCloneObject } from "./shallowClone.js";
2021
export type { IPromiseTimer, IPromiseTimerResult, ITimer } from "./timer.js";
2122
export { PromiseTimer, setLongTimeout, Timer } from "./timer.js";
2223
export { unreachableCase } from "./unreachable.js";
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*!
2+
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
3+
* Licensed under the MIT License.
4+
*/
5+
6+
/**
7+
* Shallow clone an object.
8+
*
9+
* @param value - The object to clone
10+
* @returns A shallow clone of the input value
11+
* @internal
12+
*/
13+
export function shallowCloneObject<T extends object>(value: T): T {
14+
if (Array.isArray(value)) {
15+
return [...value] as T;
16+
}
17+
return { ...value };
18+
}

packages/framework/presence/src/latestValueManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import { createEmitter } from "@fluid-internal/client-utils";
77
import type { Listenable } from "@fluidframework/core-interfaces";
8+
import { shallowCloneObject } from "@fluidframework/core-utils/internal";
89

910
import type { BroadcastControls, BroadcastControlSettings } from "./broadcastControls.js";
1011
import { OptionalBroadcastControl } from "./broadcastControls.js";
@@ -178,7 +179,7 @@ export function Latest<T extends object, Key extends string = string>(
178179
const value: InternalTypes.ValueRequiredState<T> = {
179180
rev: 0,
180181
timestamp: Date.now(),
181-
value: { ...initialValue },
182+
value: shallowCloneObject(initialValue),
182183
};
183184
const factory = (
184185
key: Key,

packages/framework/presence/src/test/latestValueManager.spec.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@
33
* Licensed under the MIT License.
44
*/
55

6+
import { strict as assert } from "node:assert";
7+
8+
import { createPresenceManager } from "../presenceManager.js";
9+
610
import { addControlsTests } from "./broadcastControlsTests.js";
11+
import { MockEphemeralRuntime } from "./mockEphemeralRuntime.js";
712

813
import type {
914
BroadcastControlSettings,
@@ -12,12 +17,14 @@ import type {
1217
} from "@fluidframework/presence/alpha";
1318
import { Latest } from "@fluidframework/presence/alpha";
1419

20+
const testWorkspaceName = "name:testWorkspaceA";
21+
1522
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
1623
function createLatestManager(
1724
presence: IPresence,
1825
valueControlSettings?: BroadcastControlSettings,
1926
) {
20-
const states = presence.getStates("name:testWorkspaceA", {
27+
const states = presence.getStates(testWorkspaceName, {
2128
camera: Latest({ x: 0, y: 0, z: 0 }, valueControlSettings),
2229
});
2330
return states.props.camera;
@@ -30,6 +37,42 @@ describe("Presence", () => {
3037
*/
3138
it("API use compiles", () => {});
3239

40+
describe("when initialized", () => {
41+
let presence: IPresence;
42+
43+
beforeEach(() => {
44+
presence = createPresenceManager(new MockEphemeralRuntime());
45+
});
46+
47+
it("can set and get empty object as initial value", () => {
48+
const states = presence.getStates(testWorkspaceName, {
49+
obj: Latest({}),
50+
});
51+
assert.deepStrictEqual(states.props.obj.local, {});
52+
});
53+
54+
it("can set and get object with properties as initial value", () => {
55+
const states = presence.getStates(testWorkspaceName, {
56+
obj: Latest({ x: 0, y: 0, z: 0 }),
57+
});
58+
assert.deepStrictEqual(states.props.obj.local, { x: 0, y: 0, z: 0 });
59+
});
60+
61+
it("can set and get empty array as initial value", () => {
62+
const states = presence.getStates(testWorkspaceName, {
63+
arr: Latest([]),
64+
});
65+
assert.deepStrictEqual(states.props.arr.local, []);
66+
});
67+
68+
it("can set and get array with elements as initial value", () => {
69+
const states = presence.getStates(testWorkspaceName, {
70+
arr: Latest([1, 2, 3]),
71+
});
72+
assert.deepStrictEqual(states.props.arr.local, [1, 2, 3]);
73+
});
74+
});
75+
3376
addControlsTests(createLatestManager);
3477
});
3578
});

0 commit comments

Comments
 (0)