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

Tiny cleanup of PureDataObject #23623

Merged
merged 8 commits into from
Jan 28, 2025
+14 −16
Merged

Tiny cleanup of PureDataObject #23623

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
826ba3b
Tiny cleanup of PureDataObject
CraigMacomber Jan 22, 2025
5585520
Cleanup initializeP
CraigMacomber Jan 22, 2025
2f811c1
Merge branch 'main' into PureDataObject
CraigMacomber Jan 22, 2025
788a716
Use JS private for initializationPromise to avoid possible name colli…
CraigMacomber Jan 22, 2025
b8297b6
Avoid removing undocumented protected initializeP property.
CraigMacomber Jan 23, 2025
891c955
Undo rename
CraigMacomber Jan 24, 2025
efa6a34
call -> await again
CraigMacomber Jan 28, 2025
7b2ed3f
Merge branch 'main' into PureDataObject
CraigMacomber Jan 28, 2025
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
2 changes: 0 additions & 2 deletions packages/framework/aqueduct/api-report/aqueduct.legacy.alpha.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ export abstract class PureDataObject<I extends DataObjectTypes = DataObjectTypes
get IFluidHandle(): IFluidHandleInternal<this>;
get IFluidLoadable(): this;
initializeInternal(existing: boolean): Promise<void>;
// (undocumented)
protected initializeP: Promise<void> | undefined;
CraigMacomber marked this conversation as resolved.
Show resolved Hide resolved
protected initializingFirstTime(props?: I["InitialState"]): Promise<void>;
protected initializingFromExisting(): Promise<void>;
// (undocumented)
Expand Down
4 changes: 1 addition & 3 deletions packages/framework/aqueduct/src/data-objects/dataObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Licensed under the MIT License.
*/

// eslint-disable-next-line import/no-deprecated
import {
type ISharedDirectory,
MapFactory,
Expand Down Expand Up @@ -48,7 +47,7 @@ export abstract class DataObject<
* Initializes internal objects and calls initialization overrides.
* Caller is responsible for ensuring this is only invoked once.
*/
public async initializeInternal(existing: boolean): Promise<void> {
public override async initializeInternal(existing: boolean): Promise<void> {
if (existing) {
// data store has a root directory so we just need to set it before calling initializingFromExisting
this.internalRoot = (await this.runtime.getChannel(
Expand All @@ -68,7 +67,6 @@ export abstract class DataObject<
}
} else {
// Create a root directory and register it before calling initializingFirstTime
// eslint-disable-next-line import/no-deprecated
this.internalRoot = SharedDirectory.create(this.runtime, this.rootDirectoryId);
this.internalRoot.bindToContext();
}
Expand Down
11 changes: 4 additions & 7 deletions packages/framework/aqueduct/src/data-objects/pureDataObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export abstract class PureDataObject<I extends DataObjectTypes = DataObjectTypes

protected initProps?: I["InitialState"];

protected initializeP: Promise<void> | undefined;
private initializationPromise: Promise<void> | undefined;

public get id(): string {
return this.runtime.id;
Expand Down Expand Up @@ -117,18 +117,15 @@ export abstract class PureDataObject<I extends DataObjectTypes = DataObjectTypes
}

/**
* Call this API to ensure PureDataObject is fully initialized.
* Await this API to ensure PureDataObject is fully initialized.
* Initialization happens on demand, only on as-needed bases.
* In most cases you should allow factory/object to decide when to finish initialization.
* But if you are supplying your own implementation of DataStoreRuntime factory and overriding some methods
* and need a fully initialized object, then you can call this API to ensure object is fully initialized.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably fine to leave. But "call ... to ensure" also appears on line 131, rather than the updated "await" text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

*/
public async finishInitialization(existing: boolean): Promise<void> {
if (this.initializeP !== undefined) {
return this.initializeP;
}
this.initializeP = this.initializeInternal(existing);
return this.initializeP;
this.initializationPromise ??= this.initializeInternal(existing);
return this.initializationPromise;
}

/**
Expand Down
9 changes: 3 additions & 6 deletions packages/test/test-utils/src/testFluidObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class TestFluidObject implements ITestFluidObject {

public root!: ISharedMap;
private readonly innerHandle: IFluidHandle<this>;
private initializeP: Promise<void> | undefined;
private initializationPromise: Promise<void> | undefined;

/**
* Creates a new TestFluidObject.
Expand Down Expand Up @@ -104,11 +104,8 @@ export class TestFluidObject implements ITestFluidObject {
this.root = (await this.runtime.getChannel("root")) as ISharedMap;
};

if (this.initializeP === undefined) {
this.initializeP = doInitialization();
}

return this.initializeP;
this.initializationPromise ??= doInitialization();
return this.initializationPromise;
}
}

Expand Down
Loading