Skip to content

improvement(client): improved internal submitMessage, reSubmit, rollback, and submitSignal type-safety #24319

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions .changeset/frowny-paths-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@fluidframework/datastore": minor
"@fluidframework/test-runtime-utils": minor
"__section": legacy
---

Deprecate submitMessage on FluidDataStoreRuntime and MockFluidDataStoreRuntime

Implementing `FluidDataStoreRuntime.submitMessage` is not required per `IFluidDataStoreChannel` and is now deprecated on `FluidDataStoreRuntime` and corresponding `MockFluidDataStoreRuntime`.

See [issue #24406](https://github.com/microsoft/FluidFramework/issues/24406) for details and alternatives.
Copy link
Member

Choose a reason for hiding this comment

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

That issue mentions:

IFluidParentContext (the base interface for IFluidDataStoreContext) should always be used to access submitMessage functionality.

That may be true for inside FluidDataStoreRuntime class, but as for the public API to submit an op, I think the comment should just be - "Submitting arbitrary messages is not supported"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is FluidDataStoreRuntime a class customers are expected to derive from? I don't think Pages does and I see that we do internally in at least two places.
Feel free edit the issue.
We should seal the class if no one should derive from it. Then it will be easier to blanketly say don't call.

3 changes: 1 addition & 2 deletions packages/common/core-interfaces/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export interface ISignalEnvelope {
*/
contents: {
type: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
content: any;
content: unknown;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class RuntimeAttributorDataStoreChannel
/**
* {@inheritdoc IFluidDataStoreChannel.reSubmit}
*/
public reSubmit(type: string, content: unknown, localOpMetadata: unknown): void {
public reSubmit(): void {
// Should not resubmit anything from the attributor as the attributor does not send ops yet.
throw new Error("Should not resubmit anything from the attributor");
}
Expand Down
Loading
Loading