Avoid exporting SharedObject as public #19717
Conversation
github-actions
bot
added
area: dds
|
FYI - @anthony-murphy and/or @ChumpChief have expressed interest in this improving this area in the past. |
github-actions
bot
added
area: tests
|
It should be working now. A lot of these changes can be pulling into a separate PR (ex: using the interfaces instead of concreate types when practical). Once that's been done, this PR will be more carefully evaluated and have its breaking changes documented. |
## Description Add TChannel type parameter to IChannelFactory, and a few related cleanups. Changes split out from #19717 ## Breaking Changes Add TChannel type parameter (which defaults to IFluidLoadable) to IChannelFactory. When left at its default this preserves the old behavior, however packages exporting IChannelFactory will now reference IFluidLoadable if not providing a different parameter and thus will implicitly depend on @fluidframework/core-interfaces SharedCounter.create now returns ISharedCounter instead of SharedCounter
github-actions
bot
removed
dependencies
| protected summarizeCore(serializer: IFluidSerializer, telemetryContext?: ITelemetryContext): ISummaryTreeWithStats; | ||
| values(): IterableIterator<any>; | ||
| } | ||
| export const SharedMap: { |
There was a problem hiding this comment.
Glad to see that the exposure of the map API has been removed. I believe we can implement similar changes for SharedDirectory, I'm willing to take care of it.
AB#5799
There was a problem hiding this comment.
SharedDirectory is already exported as alpha, so its much less urgent (so I'm not doing it), but would be great to fix. I'm glad to have someone else take that on!
|
|
||
| public static getFactory(): IChannelFactory<ITree> { | ||
| export const SharedTree = { | ||
| getFactory(): IChannelFactory<ITree> { |
There was a problem hiding this comment.
I've added some docs, though documenting his is hard since it doesn't make a lot of sense to me.
| readonly getFactory: () => IChannelFactory; | ||
| } & LoadableObjectCtor<T>; | ||
| export interface SharedObjectClass<T extends IFluidLoadable> { | ||
| readonly getFactory: () => IChannelFactory<T>; |
There was a problem hiding this comment.
This whole API surface seems a bit screwy. Here we have an interface thats implemented in packages that arn't allowed to depend on this interface.
Also the interface is a factory factory for what seems like no reason (could be just the IChannelFactory).
This could really use another follow-up breaking change that rationalizes and document is. I'll minimally patch some of the docs gaps for now in this change.
There was a problem hiding this comment.
I've added some docs, though documenting his is hard since it doesn't make a lot of sense to me.
Hide the class implementation of `SharedDirectory` and update the corresponding exports, a follow up fix of #19717 [AB#5799](https://dev.azure.com/fluidframework/internal/_workitems/edit/5799) --------- Co-authored-by: Scarlett Lee <90647596+scarlettjlee@users.noreply.github.com>
Description
Remove the requirement that factories are classes, then avoid exporting DDS classes and SharedObject (and some related types) as public: they are instead exported as
@alpha.Breaking Changes
Users of DDS concreate classes should migrate onto the corresponding interface types.
Any code using APIs intended only for authoring of new DDSes (such as SharedObject, SharedObjectCore and EventEmitterWithErrorHandling) will have to use the unstable
@alphaAPIs, and should find alternatives or be up streamed into this repository.This also makes ContainerSchema read-only: as this type is intended for defining input to the these packages this should make the APIs more tolerant and thus be non-breaking, however its possible for some users of this type to use it in ways where this could be a breaking change: any such users should remove their mutations and/or use different types.
Reviewer Guidance
The review process is outlined on this wiki page.
This makes several related changes, but as they are related and small enough that it seems reasonable for them to be grouped like this. If you would prefer them split up let me know.