Skip to content

feat(client-presence): Latest support beyond object #24417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/easy-bats-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@fluidframework/presence": minor
"__section": feature
---
Latest and LatestMap support more types

- `Latest` (`StateFactory.latest`) permits `null` so that nullable types may be used.

Check failure on line 7 in .changeset/easy-bats-type.md

View workflow job for this annotation

GitHub Actions / vale

[vale] reported by reviewdog 🐶 [Vale.Spelling] Did you really mean 'nullable'? Raw Output: {"message": "[Vale.Spelling] Did you really mean 'nullable'?", "location": {"path": ".changeset/easy-bats-type.md", "range": {"start": {"line": 7, "column": 59}}}, "severity": "ERROR"}
- `LatestMap` (`StateFactory.latestMap`) permits `boolean`, `number`, `string`, and `null`.
4 changes: 2 additions & 2 deletions packages/framework/presence/api-report/presence.alpha.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export interface Latest<T> {
}

// @alpha
export function latest<T extends object, Key extends string = string>(initialValue: JsonSerializable<T> & JsonDeserialized<T> & object, controls?: BroadcastControlSettings): InternalTypes.ManagerFactory<Key, InternalTypes.ValueRequiredState<T>, Latest<T>>;
export function latest<T extends object | null, Key extends string = string>(initialValue: JsonSerializable<T> & JsonDeserialized<T> & (object | null), controls?: BroadcastControlSettings): InternalTypes.ManagerFactory<Key, InternalTypes.ValueRequiredState<T>, Latest<T>>;

// @alpha @sealed
export interface LatestClientData<T> extends LatestData<T> {
Expand Down Expand Up @@ -186,7 +186,7 @@ export interface LatestMap<T, Keys extends string | number = string | number> {
}

// @alpha
export function latestMap<T extends object, Keys extends string | number = string | number, RegistrationKey extends string = string>(initialValues?: {
export function latestMap<T, 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, Keys>, LatestMap<T, Keys>>;

Expand Down
2 changes: 1 addition & 1 deletion packages/framework/presence/src/latestMapValueManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ class LatestMapValueManagerImpl<
* @alpha
*/
export function latestMap<
T extends object,
T,
Keys extends string | number = string | number,
RegistrationKey extends string = string,
>(
Expand Down
7 changes: 4 additions & 3 deletions packages/framework/presence/src/latestValueManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,17 @@ class LatestValueManagerImpl<T, Key extends string>
*
* @alpha
*/
export function latest<T extends object, Key extends string = string>(
initialValue: JsonSerializable<T> & JsonDeserialized<T> & object,
export function latest<T extends object | null, Key extends string = string>(
// eslint-disable-next-line @rushstack/no-new-null
initialValue: JsonSerializable<T> & JsonDeserialized<T> & (object | null),
controls?: BroadcastControlSettings,
): InternalTypes.ManagerFactory<Key, InternalTypes.ValueRequiredState<T>, Latest<T>> {
// Latest takes ownership of initialValue but makes a shallow
// copy for basic protection.
const value: InternalTypes.ValueRequiredState<T> = {
rev: 0,
timestamp: Date.now(),
value: shallowCloneObject(initialValue),
value: initialValue === null ? initialValue : shallowCloneObject(initialValue),
};
const factory = (
key: Key,
Expand Down
31 changes: 31 additions & 0 deletions packages/framework/presence/src/test/latestMapValueManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ export function checkCompiles(): void {
console.log(key, value);
}

// ----------------------------------
// pointers data

interface PointerData {
x: number;
y: number;
Expand Down Expand Up @@ -184,4 +187,32 @@ export function checkCompiles(): void {
pointers.events.on("remoteUpdated", ({ attendee, items }) => {
for (const [key, { value }] of items.entries()) logClientValue({ attendee, key, value });
});

// ----------------------------------
// primitive and null value support

workspace.add(
"primitiveMap",
StateFactory.latestMap({
// eslint-disable-next-line unicorn/no-null
null: null,
string: "string",
number: 0,
boolean: true,
}),
);

const localPrimitiveMap = workspace.props.primitiveMap.local;

// map value types are not matched to specific key
localPrimitiveMap.set("string", 1);
localPrimitiveMap.set("number", false);
// eslint-disable-next-line unicorn/no-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" });
}
26 changes: 26 additions & 0 deletions packages/framework/presence/src/test/latestValueManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { StateFactory } from "@fluidframework/presence/alpha";

const testWorkspaceName = "name:testWorkspaceA";

/* eslint-disable unicorn/no-null */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add an explanation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b/c the rule is largely dumb?
I merged so resolutions can start happening and can add comment later.
I don't really know why we have this rule on. It seems we can just use null appropriately. It does have uses and it is more annoying to deal with the rule, IMO.


// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
function createLatestManager(
presence: Presence,
Expand Down Expand Up @@ -72,13 +74,31 @@ describe("Presence", () => {
assert.deepStrictEqual(states.props.arr.local, [1, 2, 3]);
});

it("can set and get null as initial value", () => {
const states = presence.states.getWorkspace(testWorkspaceName, {
arr: StateFactory.latest(null),
});
assert.deepStrictEqual(states.props.arr.local, null);
});

it(".presence provides Presence it was created under", () => {
const states = presence.states.getWorkspace(testWorkspaceName, {
camera: StateFactory.latest({ x: 0, y: 0, z: 0 }),
});

assert.strictEqual(states.props.camera.presence, presence);
});

it("can set and get null as modified local value", () => {
// Setup
const states = presence.states.getWorkspace(testWorkspaceName, {
nullable: StateFactory.latest<{ x: number; y: number } | null>({ x: 0, y: 0 }),
});

// Act and Verify
states.props.nullable.local = null;
assert.deepStrictEqual(states.props.nullable.local, null);
});
});

addControlsTests(createLatestManager);
Expand Down Expand Up @@ -115,6 +135,7 @@ export function checkCompiles(): void {
const statesWorkspace = presence.states.getWorkspace("name:testStatesWorkspaceWithLatest", {
cursor: StateFactory.latest({ x: 0, y: 0 }),
camera: StateFactory.latest({ x: 0, y: 0, z: 0 }),
nullablePoint: StateFactory.latest<null | { x: number; y: number }>(null),
});
// Workaround ts(2775): Assertions require every name in the call target to be declared with an explicit type annotation.
const workspace: typeof statesWorkspace = statesWorkspace;
Expand All @@ -141,6 +162,9 @@ export function checkCompiles(): void {
// Update our cursor position
cursor.local = { x: 1, y: 2 };

// Set nullable point to non-null value
props.nullablePoint.local = { x: 10, y: -2 };

// Listen to others cursor updates
const cursorUpdatedOff = cursor.events.on("remoteUpdated", ({ attendee, value }) =>
console.log(`attendee ${attendee.attendeeId}'s cursor is now at (${value.x},${value.y})`),
Expand All @@ -156,3 +180,5 @@ export function checkCompiles(): void {
logClientValue({ attendee, value });
}
}

/* eslint-enable unicorn/no-null */
Loading