Skip to content

refactor(presence): Create branded JsonDeserialized type #24641

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

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

tylerbutler
Copy link
Member

Updates the presence internals to use a branded internal type when passing around JsonDeserialized objects. There are two functions to brand and unbrand a JsonDeserialized object.

Externally, the API remains the same; the internal types and functions are marked @system and objects are converted between the types at the API boundaries.

@github-actions github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct public api change Changes to a public API base: main PRs targeted against main branch labels May 15, 2025
@tylerbutler tylerbutler changed the title typebrand the JsonDeserialized type refactor(presence): Create branded JsonDeserialized type May 15, 2025
* @system
*/
export declare class JsonDeserializedBrand<T> {
private readonly JsonDeserialized: JsonDeserialized<T>;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably drop the JsonDeserialized<T>. Just T is probably enough. Probably want to use the dummy accessor and iterator found in that reference PR's brand classes.

Comment on lines 274 to 275
const unbrandedValue = fromJsonDeserializedHandle(item.value);
callbackfn(asDeeplyReadonly(unbrandedValue), key, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably combine both of these pass-through helper functions.

Comment on lines 280 to 281
const data = this.value.items[key]?.value;
return data === undefined ? undefined : asDeeplyReadonly(fromJsonDeserializedHandle(data));
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind of silly since those are both pass through anyway - should be able to adjust the helper to handle it.

@github-actions github-actions bot added area: examples Changes that focus on our examples and removed area: examples Changes that focus on our examples labels May 28, 2025
@tylerbutler tylerbutler force-pushed the presence-type-branding branch from f6391fb to 1c54887 Compare May 28, 2025 02:08
@github-actions github-actions bot removed the public api change Changes to a public API label May 28, 2025
@github-actions github-actions bot added the public api change Changes to a public API label May 28, 2025
@github-actions github-actions bot added the area: runtime Runtime related issues label May 28, 2025
Comment on lines 60 to 66
ExtensionMessage<{
type: TMessage["type"];
content: JsonSerializable<
TMessage["content"],
{ AllowExtensionOf: OpaqueJsonDeserialized<TMessage["content"]> }
>;
}>;
Copy link
Member Author

Choose a reason for hiding this comment

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

Temporary workaround to be reverted.

jason-ha and others added 5 commits May 29, 2025 13:52
Introduce `OpaqueJsonDeserialized`, and `OpaqueJsonSerializable` as "handles" for generic values.

Opaque Json types can be used when there is no direct need to work with a complex or unknown type, but still need to respect serializability. Common use is when a value that has passed through `JsonSerializable` or `JsonDeserialized` filter utilities needs to be stored, especially in a generic context. The Opaque types are recognized by `JsonSerializable` and `JsonDeserialized` allowing those stored captures to themselves be part of flow that checks for serializability or deserialization.

Test cases:
  JsonDeserialized
    positive compilation tests
      fully supported object types are preserved
        ✔ OpaqueJsonDeserialized<{number:number}>
      partially supported object types are modified
        OpaqueJsonSerialized becomes OpaqueJsonDeserialized counterpart
          ✔ OpaqueJsonSerializable<{number:number}> becomes OpaqueJsonDeserialized<{number:number}>
          ✔ OpaqueJsonSerializable<{number:number}>&OpaqueJsonDeserialized<{number:number}> becomes OpaqueJsonDeserialized<{number:number}>
        opaque Json types requiring extra allowed types have extras removed
          ✔ opaque serializable object with `bigint`
          ✔ opaque deserialized object with `bigint`
          ✔ opaque serializable and deserialized object with `bigint`
          ✔ opaque serializable object with number array expecting `bigint` support
          ✔ opaque deserialized object with number array expecting `bigint` support
          ✔ opaque serializable and deserialized object with number array expecting `bigint` support
    special cases
      using alternately allowed types
        are preserved
          ✔ opaque serializable object with `bigint`
          ✔ opaque deserialized object with `bigint`
          ✔ opaque serializable and deserialized object with `bigint`

  JsonSerializable
    positive compilation tests
      supported object types
        opaque Json types
          ✔ opaque serializable object
          ✔ opaque deserialized object
          ✔ opaque serializable and deserialized object
    negative compilation tests
      unsupported types cause compiler error
        opaque Json types requiring extra allowed types
          ✔ opaque serializable object with `bigint`
          ✔ opaque deserialized object with `bigint`
          ✔ opaque serializable and deserialized object with `bigint`
          ✔ opaque serializable object with number array expecting `bigint` support
          ✔ opaque deserialized object with number array expecting `bigint` support
          ✔ opaque serializable and deserialized object with number array expecting `bigint` support

  OpaqueJsonSerializable and OpaqueJsonDeserialized
    positive compilation tests
      ✔ OpaqueJsonSerializable is covariant (more specific is assignable to general)
      ✔ OpaqueJsonDeserialized is covariant (more specific is assignable to general)
      ✔ OpaqueJsonSerializable & OpaqueJsonDeserialized is covariant (more specific is assignable to general)
      in a generic context
        ✔ OpaqueJsonDeserialized assignability varies with JsonDeserialized Options
        ✔ OpaqueJsonSerializable assignability varies with JsonSerializable Options
        ✔ OpaqueJsonSerializable may be forwarded as JsonSerializable
        ✔ OpaqueJsonDeserialized may be returned to JsonDeserialized
        ✔ OpaqueJsonSerializable & OpaqueJsonDeserialized may be returned to JsonSerializable & JsonDeserialized
        ✔ OpaqueJsonSerializable & OpaqueJsonDeserialized may be forwarded as JsonSerializable
        ✔ OpaqueJsonSerializable & OpaqueJsonDeserialized may be returned as JsonDeserialized
    negative compilation tests
      ✔ OpaqueJsonSerializable is covariant (more general is NOT assignable to specific)
      ✔ OpaqueJsonDeserialized is covariant (more general is NOT assignable to specific)
      ✔ OpaqueJsonSerializable & OpaqueJsonDeserialized is covariant (more specific is assignable to general)
@github-actions github-actions bot removed the area: runtime Runtime related issues label May 30, 2025
tylerbutler and others added 7 commits June 2, 2025 12:51
Simplify internal unknown (generic) customer data that must be Json serializable using Opaque Json types

At boundaries of API ins and outs, Json filtered types are cast to/from Opaque Json types. See new helpers:
  - `fullySerializableToOpaqueJson`
  - `asDeserializedJson`
  - `asDeeplyReadonlyDeserializedJson`

Notifications utilities are updated and removal of `string &` for `NotificationListeners` allowed clean up of `@ts-ignore-error`. `NotificationsManagerImpl`'s `emit` implementation now just types `name` to `string`.'
Copy link
Contributor

github-actions bot commented Jun 2, 2025

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  222981 links
    1707 destination URLs
    1939 URLs ignored
       0 warnings
       0 errors


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants