Skip to content

Remove deprecated ILoaderOptions.enableOfflineLoad #27290

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

Open
dannimad wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove deprecated ILoaderOptions.enableOfflineLoad #27290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions .changeset/remove-enable-offline-load.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@fluidframework/container-definitions": minor
"@fluidframework/container-loader": minor
"__section": legacy
---

Remove deprecated `ILoaderOptions.enableOfflineLoad`

The deprecated `enableOfflineLoad` property on `ILoaderOptions` has been removed, along with the legacy `Fluid.Container.enableOfflineLoad` feature gate read from the config provider. Offline load remains enabled by default for interactive clients.

Check failure on line 9 in .changeset/remove-enable-offline-load.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'config'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'config'?", "location": {"path": ".changeset/remove-enable-offline-load.md", "range": {"start": {"line": 9, "column": 168}}}, "severity": "ERROR"}

To opt out, set `Fluid.Container.enableOfflineFull` to `false` via the config provider. To prevent silent misconfiguration, container load now throws a `UsageError` if a `pendingLocalState` is provided while `Fluid.Container.enableOfflineFull` is explicitly set to `false`.

Check failure on line 11 in .changeset/remove-enable-offline-load.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'misconfiguration'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'misconfiguration'?", "location": {"path": ".changeset/remove-enable-offline-load.md", "range": {"start": {"line": 11, "column": 107}}}, "severity": "ERROR"}

Check failure on line 11 in .changeset/remove-enable-offline-load.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'config'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'config'?", "location": {"path": ".changeset/remove-enable-offline-load.md", "range": {"start": {"line": 11, "column": 72}}}, "severity": "ERROR"}

#### Migration

- Remove any use of `enableOfflineLoad` from `ILoaderOptions` you pass to the `Loader`.
- If you previously set `Fluid.Container.enableOfflineLoad` via the config provider, set `Fluid.Container.enableOfflineFull` instead.

Check failure on line 16 in .changeset/remove-enable-offline-load.md

View workflow job for this annotation

GitHub Actions / vale 

[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'config'?

Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'config'?", "location": {"path": ".changeset/remove-enable-offline-load.md", "range": {"start": {"line": 16, "column": 69}}}, "severity": "ERROR"}
1 change: 0 additions & 1 deletion ...ommon/container-definitions/api-report/container-definitions.legacy.beta.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,6 @@ export interface ILoader extends Partial<IProvideLoader> {
export type ILoaderOptions = {
cache?: boolean;
client?: IClient;
enableOfflineLoad?: boolean;
Copy link
Copy Markdown
Contributor

@anthony-murphy anthony-murphy May 12, 2026

Choose a reason for hiding this comment

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

this is a legacy breaking change and needs to wait for the next legacy breaking release

provideScopeLoader?: boolean;
maxClientLeaveWaitTime?: number;
};
Expand Down
5 changes: 0 additions & 5 deletions packages/common/container-definitions/src/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,11 +610,6 @@ export type ILoaderOptions = {
*/
client?: IClient;

/**
* @deprecated Do not use.
*/
enableOfflineLoad?: boolean;

/**
* Provide the current Loader through the scope object when creating Containers. It is added
Copy link
Copy Markdown
Contributor

@anthony-murphy anthony-murphy May 12, 2026

Choose a reason for hiding this comment

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

Deep Review: This removes enableOfflineLoad?: boolean from the @legacy @beta ILoaderOptions surface (api-report change at container-definitions.legacy.beta.api.md:436-440). You flagged this yourself in the PR description: "This is a @legacy @beta break. Per the Beta-Break-Process it must land in a .N0 minor and may need to be staged on test/breaks/client/X.Y0/. Confirm we're in the right release window before merging."

The diff contains no entry under packages/test/breaks/client/... and no version-bump-policy / legacy.beta rename. Precedent #19306 (ChumpChief, 2024-02-02) took two steps over a long window — deprecate-then-remove — with explicit framing debate on the API surface.

Before merge:

  • Confirm with the release manager and ChumpChief that this lands in a .N0 minor for @fluidframework/container-definitions.
  • If the Beta-Break-Process harness requires it for the current version, add the corresponding test/breaks/client/X.Y0/ staging entry in this PR.
  • Verify CI's type-compat pipeline produces the correct validate*Previous.generated.ts artifacts (regenerate by hand if needed).

* as the `ILoader` property, and will overwrite an existing property of the same name on the
Expand Down
13 changes: 8 additions & 5 deletions packages/loader/container-loader/src/container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,11 +961,14 @@ export class Container
enableSummarizeProtocolTree,
);

const offlineLoadEnabled =
this.isInteractiveClient &&
(this.mc.config.getBoolean("Fluid.Container.enableOfflineLoad") ??
this.mc.config.getBoolean("Fluid.Container.enableOfflineFull") ??
options.enableOfflineLoad !== false);
const explicitOfflineFull = this.mc.config.getBoolean("Fluid.Container.enableOfflineFull");
// paranoic defense mechanism while enableOfflineFull still exists in config.
if (pendingLocalState !== undefined && explicitOfflineFull === false) {
Copy link
Copy Markdown
Contributor

@anthony-murphy anthony-murphy May 12, 2026

Choose a reason for hiding this comment

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

the user passing pending state is a signal they want to load from it, i don't think we should override that decision based on flag, and i don't think we did before. the flags are a bit of a mess, but i wouldn't expect new error cases

Copy link
Copy Markdown
Contributor Author

@dannimad dannimad May 12, 2026

Choose a reason for hiding this comment

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

It's not an override but a safeguard. In any case, given these change will need to wait I'd just ignore the depracated flag for now. I wanted to start with this one as it was the cleanest to remove. I agree flags are a mess right now.

throw new UsageError(
"Cannot load from pending local state when Fluid.Container.enableOfflineFull is explicitly disabled",
);
}
const offlineLoadEnabled = this.isInteractiveClient && (explicitOfflineFull ?? true);
Copy link
Copy Markdown
Contributor

@anthony-murphy anthony-murphy May 12, 2026

Choose a reason for hiding this comment

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

Deep Review: The new code replaces getBoolean("Fluid.Container.enableOfflineLoad") ?? getBoolean("Fluid.Container.enableOfflineFull") ?? options.enableOfflineLoad !== false with a single read of Fluid.Container.enableOfflineFull. A consumer who today sets Fluid.Container.enableOfflineLoad: false via the config provider to opt out of offline-load gets the opposite behaviour after this PR — offline silently re-enabled. The new UsageError only fires on explicitOfflineFull === false, so this case produces no UsageError, no telemetry, and no compile break (the config provider is a dynamic string surface).

PR #19306 (ChumpChief, 2024-02-02) established the transition-path rule for this exact option — anthony-murphy: "i like removing it, but i think its used today, so we need a transition path, i don't think we can just unilaterally remove". This PR satisfies that for the typed surface but not for the equally-public config-provider key. The changeset documents the rename but no runtime hint exists when the stale key is observed.

Two cheap local fixes, either works:

  • (a) Emit a one-shot mc.logger telemetry warning when Fluid.Container.enableOfflineLoad is observed in the config provider, so partners notice they need to migrate before behaviour silently flips.
  • (b) Extend the new UsageError guard to also throw when the legacy key is explicitly false, mirroring the pendingLocalState + enableOfflineFull=false safety net.

Either keeps the "single gate" goal of the PR while preserving a runtime signal for migrating consumers.

this.serializedStateManager = new SerializedStateManager(
this.subLogger,
this.storageAdapter,
Expand Down
1 change: 0 additions & 1 deletion packages/test/test-service-load/src/optionsMatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const loaderOptionsMatrix: OptionsMatrix<ILoaderOptionsExperimental> = {
client: [undefined],
provideScopeLoader: booleanCases,
maxClientLeaveWaitTime: numberCases,
enableOfflineLoad: booleanCases,
enableOfflineSnapshotRefresh: booleanCases,
snapshotRefreshTimeoutMs: [undefined, 60 * 5 * 1000 /* 5min */],
};
Expand Down
5 changes: 1 addition & 4 deletions packages/test/test-service-load/src/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,7 @@ async function scheduleOffline(
if (container.closed) {
return undefined;
}
if (
runConfig.loaderConfig?.enableOfflineLoad === true &&
random.real() < stashPercent
) {
if (random.real() < stashPercent) {
printStatus(runConfig, "closing offline container!");
const pendingState = await asLegacyAlpha(container).getPendingLocalState();
container.close();
Expand Down
Loading