Skip to content

refactor(presence): Use branded JsonDeserialized type internally #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

Closed
wants to merge 81 commits into from

Conversation

tylerbutler
Copy link
Member

@tylerbutler tylerbutler commented May 15, 2025

Updates the presence internals to use an opaque/branded internal type when passing around JSON objects. There are two functions to make JSON types opaque/not opaque, as well as a helper that casts from an opaque JSON type to a DeeplyReadonly<JsonDeserialized<T>> object since that is what we often need.

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.

There were API level mismatches with the Notifications changes, so I marked InternalUtilityTypes as alpha. Is there a better change? Similarly, the core-interfaces types needed to be beta since the presence APIs are marked as such.

@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
@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
@tylerbutler tylerbutler marked this pull request as ready for review June 4, 2025 00:17
@tylerbutler tylerbutler requested a review from a team as a code owner June 4, 2025 00:17
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like all of these are incorrect. Likely you imported from the wrong path somewhere. See the note on lines 15-18.

jason-ha and others added 22 commits June 4, 2025 01:06
…able

Filtering the inner type when applying additional `JsonSerializable` filters broken the result down into `JsonTypeWith | Opaque` degenerate substitutes when `unknown` was sufficient already and would break the filter type checking.

Now preserve the Opaque type's `T` as-is and rely on changes in `Controls` to dictate incompatibility.
This is required to allow `system:presence` workspace's datastore to deviated from the common general workspace datastore.

This also removes test workarounds when specifying `clientToSessionId` records.
…able

Filtering the inner type when applying additional `JsonSerializable` filters broken the result down into `JsonTypeWith | Opaque` degenerate substitutes when `unknown` was sufficient already and would break the filter type checking.

Now preserve the Opaque type's `T` as-is and rely on changes in `Controls` to dictate incompatibility.
This is required to allow `system:presence` workspace's datastore to deviated from the common general workspace datastore.

This also removes test workarounds when specifying `clientToSessionId` records.
…ral-datastore-typing' into presence-type-branding
- `serializableToOpaqueJson` -> `toOpaqueJson`
- `asDeserializedJson` -> `revealOpaqueJson`
Copy link
Contributor

github-actions bot commented Jun 7, 2025

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 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"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

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

> fluid-framework-docs-site@0.0.0 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


@tylerbutler
Copy link
Member Author

This change is superseded by #24772.

@tylerbutler tylerbutler closed this Jun 7, 2025
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