Skip to content
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.

Sign up for GitHub

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 redundant members of ISharedObject #10076

Merged
merged 2 commits into from
May 3, 2022
Merged

Remove redundant members of ISharedObject #10076

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f9510d
Remove redundent members of ISharedObject
Apr 27, 2022
521a568
Merge branch 'main' into dedup-shared-object
May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions api-report/shared-object-base.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,7 @@ export interface ISerializedHandle {
// @public
export interface ISharedObject<TEvent extends ISharedObjectEvents = ISharedObjectEvents> extends IChannel, IEventProvider<TEvent> {
bindToContext(): void;
connect(services: IChannelServices): void;
getAttachSummary(fullTree?: boolean, trackState?: boolean): ISummaryTreeWithStats;
getGCData(fullGC?: boolean): IGarbageCollectionData;
isAttached(): boolean;
summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;
}

// @public (undocumented)
Expand All @@ -88,11 +84,13 @@ export function serializeHandles(value: any, serializer: IFluidSerializer, bind:
// @public
export abstract class SharedObject<TEvent extends ISharedObjectEvents = ISharedObjectEvents> extends SharedObjectCore<TEvent> {
constructor(id: string, runtime: IFluidDataStoreRuntime, attributes: IChannelAttributes);
// (undocumented)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have been unable to fix that these are now considered "undocumented". I also do not know how to see and generated documentation produced from this code, nor do the inheritDoc tags work in vs code, so I have no way to test or use the generated documentation.

Copy link
Contributor Author

@CraigMacomber CraigMacomber May 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I can tell, this file is the only thing currently consuming the .api.json file.

It's unclear to me if this documentation being omitted here is a good thing or a bad thing:
The entire entry for these methods is omitted from the .api.json file. This could mean that it detected that the base method from the interface is sufficient to describe it, which I think would be a good thing. Alternativity it could mean that I broke the documentation.

Either way, we don't have anything that consumes uses that documentation, and vs-code intelisense is broken due to there being an inherit doc comment in the first place, so that did not regress.

Thus the one thing I use (vs code intelisense) is unchanged (still broken), and the rest is ambiguously better or worse. Thus I guess it should be fine to make this change?

Copy link
Member

Choose a reason for hiding this comment

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

This is probably because the inheritDoc reference now crosses a package boundary, which doesn't work today: microsoft/rushstack#1195

If an @inheritdoc tag refers to an external package, API Extractor does not process it. But API Documenter will process it, if it can find the target in one of its .api.json files.

Copy link
Member

Choose a reason for hiding this comment

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

The entire entry for these methods is omitted from the .api.json file. This could mean that it detected that the base method from the interface is sufficient to describe it, which I think would be a good thing.

The API report should not include inherited members, so this is correct.

getAttachSummary(fullTree?: boolean, trackState?: boolean): ISummaryTreeWithStats;
getGCData(fullGC?: boolean): IGarbageCollectionData;
protected processGCDataCore(serializer: SummarySerializer): void;
// (undocumented)
protected get serializer(): IFluidSerializer;
// (undocumented)
summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;
protected abstract summarizeCore(serializer: IFluidSerializer): ISummaryTreeWithStats;
}
Expand All @@ -104,10 +102,12 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
// (undocumented)
readonly attributes: IChannelAttributes;
bindToContext(): void;
// (undocumented)
connect(services: IChannelServices): void;
get connected(): boolean;
protected didAttach(): void;
protected dirty(): void;
// (undocumented)
abstract getAttachSummary(fullTree?: boolean, trackState?: boolean): ISummaryTreeWithStats;
abstract getGCData(fullGC?: boolean): IGarbageCollectionData;
readonly handle: IFluidHandle;
Expand All @@ -118,6 +118,7 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
get IFluidLoadable(): this;
initializeLocal(): void;
protected initializeLocalCore(): void;
// (undocumented)
isAttached(): boolean;
load(services: IChannelServices): Promise<void>;
protected abstract loadCore(services: IChannelStorageService): Promise<void>;
Expand All @@ -130,6 +131,7 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
// (undocumented)
protected runtime: IFluidDataStoreRuntime;
protected submitLocalMessage(content: any, localOpMetadata?: unknown): void;
// (undocumented)
abstract summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;
}

Expand Down
12 changes: 6 additions & 6 deletions packages/dds/shared-object-base/src/sharedObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,27 +185,27 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
}

/**
* {@inheritDoc (ISharedObject:interface).connect}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).connect}
*/
public connect(services: IChannelServices) {
this.services = services;
this.attachDeltaHandler();
}

/**
* {@inheritDoc (ISharedObject:interface).isAttached}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).isAttached}
*/
public isAttached(): boolean {
return this.services !== undefined && this.runtime.attachState !== AttachState.Detached;
}

/**
* {@inheritDoc (ISharedObject:interface).getAttachSummary}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).getAttachSummary}
*/
public abstract getAttachSummary(fullTree?: boolean, trackState?: boolean): ISummaryTreeWithStats;

/**
* {@inheritDoc (ISharedObject:interface).summarize}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).summarize}
*/
public abstract summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;

Expand Down Expand Up @@ -472,14 +472,14 @@ export abstract class SharedObject<TEvent extends ISharedObjectEvents = ISharedO
}

/**
* {@inheritDoc (ISharedObject:interface).getAttachSummary}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).getAttachSummary}
*/
public getAttachSummary(fullTree: boolean = false, trackState: boolean = false): ISummaryTreeWithStats {
return this.summarizeCore(this.serializer);
}

/**
* {@inheritDoc (ISharedObject:interface).summarize}
* {@inheritDoc @fluidframework/datastore-definitions#(IChannel:interface).summarize}
*/
public async summarize(fullTree: boolean = false, trackState: boolean = false): Promise<ISummaryTreeWithStats> {
return this.summarizeCore(this.serializer);
Expand Down
29 changes: 2 additions & 27 deletions packages/dds/shared-object-base/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
*/

import { IErrorEvent, IEventProvider, IEventThisPlaceHolder } from "@fluidframework/common-definitions";
import { IChannel, IChannelServices } from "@fluidframework/datastore-definitions";
import { IChannel } from "@fluidframework/datastore-definitions";
import { ISequencedDocumentMessage } from "@fluidframework/protocol-definitions";
import { IGarbageCollectionData, ISummaryTreeWithStats } from "@fluidframework/runtime-definitions";
import { IGarbageCollectionData } from "@fluidframework/runtime-definitions";

export interface ISharedObjectEvents extends IErrorEvent {
(event: "pre-op" | "op",
Expand All @@ -24,31 +24,6 @@ export interface ISharedObject<TEvent extends ISharedObjectEvents = ISharedObjec
*/
bindToContext(): void;

/**
* Returns whether the given shared object is attached to storage.
* @returns True if the given shared object is attached
*/
isAttached(): boolean;

/**
* Generates summary of the channel synchronously.
* @returns A tree representing the summary of the shared object.
*/
getAttachSummary(fullTree?: boolean, trackState?: boolean): ISummaryTreeWithStats;

/**
* Generates summary of the shared object asynchronously.
* This should not be called where the object can be modified while summarization is in progress.
* @returns A tree representing the summary of the channel.
*/
summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;

/**
* Enables the channel to send and receive ops.
* @param services - Services to connect to
*/
connect(services: IChannelServices): void;

/**
* Returns the GC data for this shared object. It contains a list of GC nodes that contains references to
* other GC nodes.
Expand Down
6 changes: 4 additions & 2 deletions packages/runtime/datastore-definitions/src/channel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ export interface IChannel extends IFluidLoadable {
summarize(fullTree?: boolean, trackState?: boolean): Promise<ISummaryTreeWithStats>;

/**
* True if the data structure is attached to storage.
* Checks if the channel is attached to storage.
* @returns True iff the channel is attached.
*/
isAttached(): boolean;

/**
* Enables the channel to send and receive ops
* Enables the channel to send and receive ops.
* @param services - Services to connect to
*/
connect(services: IChannelServices): void;

Expand Down