microsoft / FluidFramework

Library for building distributed, real-time collaborative web applications
https://fluidframework.com
MIT License
4.73k stars 532 forks source link

feat(client): Experimental presence implementation #22475

Closed jason-ha closed 1 month ago

jason-ha commented 1 month ago

Base line support for declarative model support, creating Presence States workspaces, and interacting with them.

PresenceEvents events are not connected.

Pending update: Update ISessionClient to use a stable id for lifetime and coordinate using system:presence priorClientIds (likely renamed).

Tested manually with modifications to examples/service-clients/azure-client/external-controller not shown here.

msfluid-bot commented 1 month ago
@fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 459.93 KB 459.96 KB +35 Bytes
azureClient.js 557.86 KB 557.91 KB +49 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 260.69 KB 260.7 KB +14 Bytes
fluidFramework.js 401 KB 401.01 KB +14 Bytes
loader.js 134.19 KB 134.2 KB +14 Bytes
map.js 42.43 KB 42.44 KB +7 Bytes
matrix.js 146.58 KB 146.58 KB +7 Bytes
odspClient.js 525.21 KB 525.26 KB +49 Bytes
odspDriver.js 97.8 KB 97.82 KB +21 Bytes
odspPrefetchSnapshot.js 42.76 KB 42.78 KB +14 Bytes
sharedString.js 163.3 KB 163.31 KB +7 Bytes
sharedTree.js 391.46 KB 391.47 KB +7 Bytes
Total Size 3.3 MB 3.3 MB +245 Bytes

Baseline commit: e64c6a1822672379b44c280a51cbdc68ef6210a9

Generated by :no_entry_sign: dangerJS against f92fa29d628269210db1d67203b5bbd2c584cc2a

github-actions[bot] commented 1 month ago

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluidframework-docs@0.25.0 ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"

> fluidframework-docs@0.25.0 ci:start
> http-server ./public --port 1313 --silent

> fluidframework-docs@0.25.0 linkcheck:full
> npm run linkcheck:fast -- --external

> fluidframework-docs@0.25.0 linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

Stats:
  402260 links
    3130 destination URLs
       2 URLs ignored
       0 warnings
       0 errors
jason-ha commented 1 month ago

I broke the API changes out to #22481 - with that merged first fluid-cr-api should get removed here.

jason-ha commented 1 month ago

I'm going OOF soon and haven't been able to give this a full look, but wanted to add a few quick comments. As you're still iterating, definitely no need to block on me in the meantime.

You mention having an example privately available - would be good to get it in the repo to help illustrate usage, as I think this will yield better CR feedback as we'll be able to easily try it out and understand the usage patterns.

Two problems with example:

  1. best case is with package using encapsulated API and I was not adding that support in yet. (Most examples are encapulated :( ) The prototype has support and you can see some use in the prototype example PR: https://github.com/jason-ha/FluidFramework/pull/3/files The states API is largely unchanged - main difference is using a clientId string versus ISessionClient for association. I can draft an encapsulated example PR once this PR merges.
  2. other case is external controller dice roller and I seem to have misplaced what I thought was reasonable to demo with it.

Yeah - not really my preferred roll-out, but don't think deferring until next release is helpful.

jason-ha commented 1 month ago

Looked through as much as I can (mostly as a learning exercise to be honest) and it looks experimental ready from what I can tell. Most of my comments are about adding more comments and example usage, which can be iterated on.

Still looking through for minor changes, but I will preapprove for when @ChumpChief requested changes are resolved (I saw he had some regarding forced refresh interval).

I did address all of that. Learning is really supposed to be a top thing. We however emphasize the correctness aspect too often because we don't have other things in place. Like there aren't runtime tests of these changes! We'll fix that. And we are free to edit and comment on this code at any time. It is fair enough to say looks reasonable for this project at this stage.