-
Notifications
You must be signed in to change notification settings - Fork 536
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
Remove several types from @public
scope
#21142
Conversation
@@ -75,7 +75,7 @@ export interface ContainerWarning extends IErrorBase_2 { | |||
export interface IAudience extends IEventProvider<IAudienceEvents> { | |||
getMember(clientId: string): IClient | undefined; | |||
getMembers(): Map<string, IClient>; | |||
getSelf: () => ISelf | undefined; | |||
getSelf(): ISelf | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when reviewing public APIs, I spotted this. I don't think the intention in this API was to have a mutable callback users could reassign OR call without the Audience object as its this
. At least one implementation implemented this with a method, so I made the API match.
readonly client?: IClient; | ||
readonly clientId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
being able to reassign who self is seems like not something we support, do I cleaned this up too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never had any issue with the content of this PR. It is I think the best we can do. The thing that caught my attention was the mention of encapsulated API. It is just still fuzzy. If any of those changed are actually meant to be internal then I wouldn't say they are part of encapsulated API [surface].
## Description Due to my previous changes to make IFluidDataStoreRuntime non-public, several types no longer need to be public. This moves more of them to alpha. Also includes a couple of `IAudience` related tweaks to make the types better convey what I I think is supported as far as method binding and modification. ## Breaking Changes The following types have been moved from `@public` to `@alpha`: - `IFluidSerializer` - `ISharedObjectEvents` - `IChannelServices` - `IChannelStorageService` - `IDeltaConnection` - `IDeltaHandler` These should not be needed by users of the declarative API, which is what `@public` is targeting.
Description
Due to my previous changes to make IFluidDataStoreRuntime non-public, several types no longer need to be public. This moves more of them to alpha.
Also includes a couple of
IAudience
related tweaks to make the types better convey what I I think is supported as far as method binding and modification.Breaking Changes
The following types have been moved from
@public
to@alpha
:IFluidSerializer
ISharedObjectEvents
IChannelServices
IChannelStorageService
IDeltaConnection
IDeltaHandler
These should not be needed by users of the declarative API, which is what
@public
is targeting.Reviewer Guidance
The review process is outlined on this wiki page.