-
Notifications
You must be signed in to change notification settings - Fork 532
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
Don't allow FlushModeExperimental.Async
if the loader does not support reference sequence numbers
#14239
Don't allow FlushModeExperimental.Async
if the loader does not support reference sequence numbers
#14239
Conversation
@@ -184,6 +184,9 @@ export interface IContainerContext extends IDisposable { | |||
getLoadedFromVersion(): IVersion | undefined; | |||
|
|||
updateDirtyContainerState(dirty: boolean): void; | |||
|
|||
readonly supportedFeatures?: ReadonlyMap<string, unknown>; |
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.
(Mostly talking aloud)
I believe it's right approach, but I do not know how to think about documentation here.
I.e. we should fully support parallel implementations of IRuntime, or vice versa - IContainerContext, including by 3rd parties. It's not very clear what needs to be done here RE properly documenting all the properties we are exposing here, as their timeframe (i.e. I totally expect exposure of features to be "sunset" once we have 100% saturation of new bits, i.e. the need to check for feature presents to go away)
⯅ @fluid-example/bundle-size-tests: +498 Bytes
Baseline commit: 3132fbd |
Description
With
FlushMode==FlushModeExperimental.Async
, batches will be flushed across JS turns, drastically increasing the chances of ending up in states with mismatched reference sequence numbers in the same batch. This change #14150 alleviates the risk, by flushing batches earlier. However, as old loader do not receive the reference sequence numbers from the runtime, there is considerable risk of mixed batches in that case (new runtime + FlushModeExperimental.Async - old loader). Therefore, we need to explicitly block this combination.Loaders from versions older than the present will not work with
FlushModeExperimental.Async
as they don't specify it's supported features in the ContainerContext implementation at all.