Skip to content

Commit 905e663

Browse files
authored
Runtime: Begin to deprecate Compat Details on ContainerRuntime messages (#21355)
This last year we added support to include "compat details" on an op, with the one feature there being the ability to say "it's ok if you don't recognize this op type, just ignore it". We added this to try to speed up early development of features that introduce new op types - GC in particular. But upon further reflection, this is a very dubious prospect. It glosses over a ton of complex cases that end up making little sense. Not worth any perceived utility. So we are deprecating it and will soon remove it.
1 parent 787bbe7 commit 905e663

File tree

9 files changed

+31
-53
lines changed

9 files changed

+31
-53
lines changed

packages/runtime/container-runtime/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,10 @@
287287
},
288288
"InterfaceDeclaration_IGCNodeUpdatedProps": {
289289
"forwardCompat": false
290+
},
291+
"RemovedInterfaceDeclaration_ContainerRuntimeMessage": {
292+
"backCompat": false,
293+
"forwardCompat": false
290294
}
291295
}
292296
}

packages/runtime/container-runtime/src/gc/garbageCollection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ export class GarbageCollector implements IGarbageCollector {
746746
const containerGCMessage: ContainerRuntimeGCMessage = {
747747
type: ContainerMessageType.GC,
748748
contents,
749-
compatDetails: { behavior: "Ignore" },
749+
compatDetails: { behavior: "Ignore" }, // DEPRECATED: For temporary back compat only
750750
};
751751
this.submitMessage(containerGCMessage);
752752
return;
@@ -1095,7 +1095,7 @@ export class GarbageCollector implements IGarbageCollector {
10951095
type: GarbageCollectionMessageType.TombstoneLoaded,
10961096
nodePath,
10971097
},
1098-
compatDetails: { behavior: "Ignore" },
1098+
compatDetails: { behavior: "Ignore" }, // DEPRECATED: For temporary back compat only
10991099
};
11001100
this.submitMessage(containerGCMessage);
11011101
}

packages/runtime/container-runtime/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export {
2525
} from "./containerRuntime.js";
2626
export {
2727
ContainerMessageType,
28-
ContainerRuntimeMessage,
2928
IContainerRuntimeMessageCompatDetails,
3029
CompatModeBehavior,
3130
RecentlyAddedContainerRuntimeMessageDetails,

packages/runtime/container-runtime/src/messageTypes.ts

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export enum ContainerMessageType {
6060
/**
6161
* How should an older client handle an unrecognized remote op type?
6262
*
63+
* @deprecated The utility of a mechanism to handle unknown messages is outweighed by the nuance required to get it right.
6364
* @internal
6465
*/
6566
export type CompatModeBehavior =
@@ -71,6 +72,7 @@ export type CompatModeBehavior =
7172
/**
7273
* All the info an older client would need to know how to handle an unrecognized remote op type
7374
*
75+
* @deprecated The utility of a mechanism to handle unknown messages is outweighed by the nuance required to get it right.
7476
* @internal
7577
*/
7678
export interface IContainerRuntimeMessageCompatDetails {
@@ -85,8 +87,7 @@ export interface IContainerRuntimeMessageCompatDetails {
8587
* IMPORTANT: when creating one to be serialized, set the properties in the order they appear here.
8688
* This way stringified values can be compared.
8789
*/
88-
interface TypedContainerRuntimeMessage<TType extends ContainerMessageType, TContents>
89-
extends Partial<RecentlyAddedContainerRuntimeMessageDetails> {
90+
interface TypedContainerRuntimeMessage<TType extends ContainerMessageType, TContents> {
9091
/** Type of the op, within the ContainerRuntime's domain */
9192
type: TType;
9293
/** Domain-specific contents, interpreted according to the type */
@@ -95,6 +96,7 @@ interface TypedContainerRuntimeMessage<TType extends ContainerMessageType, TCont
9596

9697
/**
9798
* Additional details expected for any recently added message.
99+
* @deprecated The utility of a mechanism to handle unknown messages is outweighed by the nuance required to get it right.
98100
* @internal
99101
*/
100102
export interface RecentlyAddedContainerRuntimeMessageDetails {
@@ -137,7 +139,9 @@ export type ContainerRuntimeIdAllocationMessage = TypedContainerRuntimeMessage<
137139
export type ContainerRuntimeGCMessage = TypedContainerRuntimeMessage<
138140
ContainerMessageType.GC,
139141
GarbageCollectionMessage
140-
>;
142+
> &
143+
// While deprecating: GC messages may still contain compat details for now
144+
Partial<RecentlyAddedContainerRuntimeMessageDetails>;
141145
export type ContainerRuntimeDocumentSchemaMessage = TypedContainerRuntimeMessage<
142146
ContainerMessageType.DocumentSchemaChange,
143147
IDocumentSchemaChangeMessage
@@ -232,23 +236,3 @@ export type InboundSequencedContainerRuntimeMessageOrSystemMessage =
232236
*/
233237
export type InboundSequencedRecentlyAddedContainerRuntimeMessage = ISequencedDocumentMessage &
234238
Partial<RecentlyAddedContainerRuntimeMessageDetails>;
235-
236-
/**
237-
* The unpacked runtime message / details to be handled or dispatched by the ContainerRuntime
238-
*
239-
* IMPORTANT: when creating one to be serialized, set the properties in the order they appear here.
240-
* This way stringified values can be compared.
241-
*
242-
* @deprecated this is an internal type which should not be used outside of the package.
243-
* Internally, it is superseded by `TypedContainerRuntimeMessage`.
244-
*
245-
* @internal
246-
*/
247-
export interface ContainerRuntimeMessage {
248-
/** Type of the op, within the ContainerRuntime's domain */
249-
type: ContainerMessageType;
250-
/** Domain-specific contents, interpreted according to the type */
251-
contents: any;
252-
/** Info describing how to handle this op in case the type is unrecognized (default: fail to process) */
253-
compatDetails?: IContainerRuntimeMessageCompatDetails;
254-
}

packages/runtime/container-runtime/src/test/containerRuntime.spec.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
} from "../containerRuntime.js";
6565
import {
6666
ContainerMessageType,
67+
type ContainerRuntimeGCMessage,
6768
type OutboundContainerRuntimeMessage,
6869
type RecentlyAddedContainerRuntimeMessageDetails,
6970
type UnknownContainerRuntimeMessage,
@@ -75,6 +76,12 @@ import {
7576
} from "../pendingStateManager.js";
7677
import { ISummaryCancellationToken, neverCancelledSummaryToken } from "../summary/index.js";
7778

79+
// Type test:
80+
const outboundMessage: OutboundContainerRuntimeMessage =
81+
{} as unknown as OutboundContainerRuntimeMessage;
82+
// @ts-expect-error Outbound type should not include compat behavior
83+
(() => {})(outboundMessage.compatDetails);
84+
7885
function submitDataStoreOp(
7986
runtime: Pick<ContainerRuntime, "submitMessage">,
8087
id: string,
@@ -973,7 +980,7 @@ describe("Runtime", () => {
973980
);
974981
});
975982

976-
describe("Future op type compatibility", () => {
983+
describe("[DEPRECATED] Future op type compatibility", () => {
977984
let containerRuntime: ContainerRuntime;
978985
beforeEach(async () => {
979986
containerRuntime = await ContainerRuntime.loadRuntime({
@@ -988,7 +995,7 @@ describe("Runtime", () => {
988995
});
989996
});
990997

991-
it("can submit op compat behavior", async () => {
998+
it("can submit op compat behavior (temporarily still available for GC op)", async () => {
992999
// Create a container runtime type where the submit method is public. This makes it easier to test
9931000
// submission and processing of ops. The other option is to send data store or alias ops whose
9941001
// processing requires creation of data store context and runtime as well.
@@ -1002,22 +1009,16 @@ describe("Runtime", () => {
10021009
const containerRuntimeWithSubmit =
10031010
containerRuntime as unknown as ContainerRuntimeWithSubmit;
10041011

1005-
const runtimeCompatMessage: Omit<
1006-
OutboundContainerRuntimeMessage,
1007-
"type" | "contents"
1008-
> & {
1009-
type: string;
1010-
contents: any;
1011-
} = {
1012-
type: "NEW",
1013-
contents: "Hello",
1012+
const gcMessageWithDeprecatedCompatDetails: ContainerRuntimeGCMessage = {
1013+
type: ContainerMessageType.GC,
1014+
contents: { type: "Sweep", deletedNodeIds: [] },
10141015
compatDetails: { behavior: "Ignore" },
10151016
};
10161017

10171018
assert.doesNotThrow(
10181019
() =>
10191020
containerRuntimeWithSubmit.submit(
1020-
runtimeCompatMessage as OutboundContainerRuntimeMessage,
1021+
gcMessageWithDeprecatedCompatDetails,
10211022
undefined,
10221023
undefined,
10231024
),

packages/runtime/container-runtime/src/test/types/validateContainerRuntimePrevious.generated.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -248,28 +248,16 @@ use_old_ClassDeclaration_ContainerRuntime(
248248
* If this test starts failing, it indicates a change that is not forward compatible.
249249
* To acknowledge the breaking change, add the following to package.json under
250250
* typeValidation.broken:
251-
* "InterfaceDeclaration_ContainerRuntimeMessage": {"forwardCompat": false}
251+
* "RemovedInterfaceDeclaration_ContainerRuntimeMessage": {"forwardCompat": false}
252252
*/
253-
declare function get_old_InterfaceDeclaration_ContainerRuntimeMessage():
254-
TypeOnly<old.ContainerRuntimeMessage>;
255-
declare function use_current_InterfaceDeclaration_ContainerRuntimeMessage(
256-
use: TypeOnly<current.ContainerRuntimeMessage>): void;
257-
use_current_InterfaceDeclaration_ContainerRuntimeMessage(
258-
get_old_InterfaceDeclaration_ContainerRuntimeMessage());
259253

260254
/*
261255
* Validate backward compatibility by using the current type in place of the old type.
262256
* If this test starts failing, it indicates a change that is not backward compatible.
263257
* To acknowledge the breaking change, add the following to package.json under
264258
* typeValidation.broken:
265-
* "InterfaceDeclaration_ContainerRuntimeMessage": {"backCompat": false}
259+
* "RemovedInterfaceDeclaration_ContainerRuntimeMessage": {"backCompat": false}
266260
*/
267-
declare function get_current_InterfaceDeclaration_ContainerRuntimeMessage():
268-
TypeOnly<current.ContainerRuntimeMessage>;
269-
declare function use_old_InterfaceDeclaration_ContainerRuntimeMessage(
270-
use: TypeOnly<old.ContainerRuntimeMessage>): void;
271-
use_old_InterfaceDeclaration_ContainerRuntimeMessage(
272-
get_current_InterfaceDeclaration_ContainerRuntimeMessage());
273261

274262
/*
275263
* Validate forward compatibility by using the old type in place of the current type.

packages/test/test-end-to-end-tests/src/test/batching.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ describeCompat("Flushing ops", "NoCompat", (getTestObjectProvider, apis) => {
148148
await provider.ensureSynchronized();
149149
}
150150

151-
it("can send and a batch containing a future/unknown op type", async () => {
151+
it("[DEPRECATED] can send and a batch containing a future/unknown op type", async () => {
152152
await setupContainers({
153153
flushMode: FlushMode.TurnBased,
154154
compressionOptions: {

packages/test/test-end-to-end-tests/src/test/stashedOps.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ describeCompat("stashed ops", "NoCompat", (getTestObjectProvider, apis) => {
376376
const string = await d.getSharedObject<SharedString>(stringId);
377377
const collection = string.getIntervalCollection(collectionId);
378378
collection.add({ start: testStart, end: testEnd });
379+
// [DEPRECATED]
379380
// Submit a message with an unrecognized type
380381
// Super rare corner case where you stash an op and then roll back to a previous runtime version that doesn't recognize it
381382
(

packages/test/test-service-load/src/loadTestDataStore.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ class LoadTestDataStore extends DataObject implements ILoadTest {
652652
largeOpsSent++;
653653
}
654654

655+
// [DEPRECATED] This flow is deprecated and is expected to be removed from FF soon.
655656
if (futureOpPeriod !== undefined && opsSent % futureOpPeriod === 0) {
656657
(
657658
this.context.containerRuntime as unknown as {

0 commit comments

Comments
 (0)