fix(client-presence): move validated State objects to internal

[jason-ha]

Some validation related type remain exposed as the *Raw types reference them.

[Copilot]

Pull Request Overview

This pull request refactors the creation of presence State objects by moving validated (raw) state implementations to an internal API.

• Refactors factory functions to use StateFactoryInternal instead of StateFactory.
• Updates test imports and usage accordingly.
• Adjusts API definitions and documentation in the public API reports.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/framework/presence/src/test/schemaValidation.spec.ts	Test updates to import and use StateFactoryInternal instead of the public API.
packages/framework/presence/src/test/latestValueManager.spec.ts	Updated test cases using StateFactoryInternal for latest state creation.
packages/framework/presence/src/test/latestMapValueManager.spec.ts	Updated test cases using StateFactoryInternal for latestMap state creation.
packages/framework/presence/src/stateFactory.ts	Refactored exports to expose internal state factory methods and updated documentation comments.
packages/framework/presence/src/latestValueManager.ts	Adjusted factory function overloads and error handling for validator usage.
packages/framework/presence/src/latestMapValueManager.ts	Updated overload signatures and helper function for argument casting.
packages/framework/presence/src/index.ts	Modified type re-exports to remove deprecated types from the public API.
packages/framework/presence/api-report/*	Synchronized API report documentation with internal state changes.
.changeset/green-poets-run.md	Removed changeset details reflecting breaking changes to the validator parameter.

Comments suppressed due to low confidence (6)

packages/framework/presence/src/latestValueManager.ts:329

• [nitpick] Consider adding a TODO or inline comment noting the planned timeline for validator support to aid future maintenance.

	}

packages/framework/presence/src/test/schemaValidation.spec.ts:14

• [nitpick] Ensure test cases fully cover the new StateFactoryInternal usage, particularly in scenarios where validator behavior might be extended in the future.

import { StateFactoryInternal } from "../stateFactory.js";

packages/framework/presence/src/stateFactory.ts:28

• Update the documentation comment to clearly indicate that StateFactory re-exports internal APIs to prevent accidental exposure of raw validated state behaviors.

export const StateFactory: {

packages/framework/presence/src/latestMapValueManager.ts:680

• [nitpick] Add an inline comment in or above the usableArgsType function to clarify its purpose for casting validator arguments, improving maintainability.

	const validator = usableArgsType(args)?.validator;

packages/framework/presence/src/index.ts:51

• Verify that deprecating the export of LatestMapArguments and LatestArguments does not adversely affect existing consumers, and update migration guides as necessary.

	// LatestMapArguments,

[jason-ha]

These comments didn't actually go anywhere. Moved to remarks in overall variable.

[github-actions]

🔗 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:
  225447 links
    1710 destination URLs
    1941 URLs ignored
       0 warnings
       0 errors

[tylerbutler]

Looks good for merge, so approving.

Trying to make sure I understand the patterns you're using here, though, so I can apply them correctly. It seems that:

1. Instead of using function overloads, you created internal and external interfaces with different methods to represent the overloads. I find this a little confusing but I am getting used to it. I think it’s just the added layers of “producers”. There’s an interface that represents a function that returns a function etc. I imagine this makes extensibility easier via the interfaces rather than messing around with function overloads and ordering, though.
2. You explicitly typed the latest and latestMap functions as const with the interface types created earlier. So now there are no fuction overloads - everything is represented by the interfaces.
3. You further separated the interface overloads via StateFactoryInternal so that the public StateFactory only exposes the intended overloads right now.

It seems like this pattern is more flexible than the overloads approach I was taking, regardless of our need to hide the API right now. That is, I think this is a superior approach than function overloads full-stop. Am I missing something else about the tradeoffs?

Is this something that made less sense when they were free functions rather than being exposed off the StateFactory? I guess those free functions could also have been consts with a type.

Anyway, thanks for doing this!

[tylerbutler]

I'm confused why the intersection works. Is it because we know we only ever pass one or the other? I guess it's not a runtime problem unless there's some type-guarding going on based on properties in one or the other that are mutually exclsuive, but that's not the case here.

[jason-ha]

I am not sure this is where things end up or if there is something more specific here.
We can "get away" with this intersection because the implementation is listed as implementing both:

class LatestValueManagerImpl<T, Key extends string>
	implements
		LatestRaw<T>,
		Latest<T>,

that is the original lie. It will really be one or the other based on validator existing. Since we are runtime consistent I didn't worry about it for this change.

[tylerbutler]

that is the original lie.

Ahhhh, yes. I see now.

[Josmithr]

Approving API changes

[jason-ha]

Looks good for merge, so approving.

Trying to make sure I understand the patterns you're using here, though, so I can apply them correctly. It seems that:

1. Instead of using function overloads, you created internal and external interfaces with different methods to represent the overloads. I find this a little confusing but I am getting used to it. I think it’s just the added layers of “producers”. There’s an interface that represents a function that returns a function etc. I imagine this makes extensibility easier via the interfaces rather than messing around with function overloads and ordering, though.
2. You explicitly typed the latest and latestMap functions as const with the interface types created earlier. So now there are no fuction overloads - everything is represented by the interfaces.
3. You further separated the interface overloads via StateFactoryInternal so that the public StateFactory only exposes the intended overloads right now.

It seems like this pattern is more flexible than the overloads approach I was taking, regardless of our need to hide the API right now. That is, I think this is a superior approach than function overloads full-stop. Am I missing something else about the tradeoffs?

Is this something that made less sense when they were free functions rather than being exposed off the StateFactory? I guess those free functions could also have been consts with a type.

Anyway, thanks for doing this!

There are multiple ways to express some things in TypeScript. functions are one of those. In EcmaScript a function is really a callable object. It is an object that has an "unnamed" property.

type foo = (args) => return

is the same as

interface foo {
 (args): return;
}

There is a lint rule (@typescript-eslint/prefer-function-type) that prefers type form. I suppressed that because it will be cleaner to evolve to overloaded with interface pattern. You can still overload with type, but it is not as nice to document IMO. See expose "validator" commit (The diff doesn't look great because...)
...I do keep the overload order with more specific first in the evolved interface change.

Really it is still function overload; just expressed with the types extracted which allows for control of what is exposed. We don't have support in our API partitioning to tag individual overloads. (I think api-extractor might be able to do that because it redeclares types. But type redeclaration causes other problems and is the reason we have our own partitioning.)