Remove deprecated ILoaderOptions.enableOfflineLoad by dannimad · Pull Request #27290 · microsoft/FluidFramework

dannimad · 2026-05-12T21:15:08Z

Description

Removes the deprecated enableOfflineLoad field from ILoaderOptions (@fluidframework/container-definitions) and the legacy Fluid.Container.enableOfflineLoad feature-gate read from the config provider. The field was already marked @deprecated Do not use.; this completes its removal.

Offline-load default behavior is preserved: interactive clients still get offline load enabled by default. To opt out, set Fluid.Container.enableOfflineFull: false via the config provider — that is now the single control surface.

As a safety net for callers migrating from the legacy gate, Container load now throws a UsageError if pendingLocalState is provided while Fluid.Container.enableOfflineFull is explicitly set to false. This prevents silently loading from pending state with offline-load disabled.

Also drops the deprecated entry from the test-service-load options matrix and updates the stress-runner gate that referenced it.

Breaking Changes

ILoaderOptions.enableOfflineLoad (@legacy @beta) is removed. Migration:

Remove enableOfflineLoad from any ILoaderOptions passed to the Loader.
If you previously set Fluid.Container.enableOfflineLoad via the config provider, set Fluid.Container.enableOfflineFull instead (same boolean value).

See Breaking-vs-Non-breaking-Changes.

Reviewer Guidance

The review process is outlined on this wiki page.

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.
fluid-cr-api will be auto-assigned as a required reviewer.
The new throw-on-misconfig is a defensive guard while enableOfflineFull remains the control surface; happy to drop or relax it if reviewers prefer.

Removes the deprecated `enableOfflineLoad` field from `ILoaderOptions` and the legacy `Fluid.Container.enableOfflineLoad` config gate. Default offline-load behavior for interactive clients is preserved; opt-out is via `Fluid.Container.enableOfflineFull: false`. Container load now throws a `UsageError` if `pendingLocalState` is provided while `Fluid.Container.enableOfflineFull` is explicitly disabled, to prevent silent offline-load misconfiguration. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

github-actions · 2026-05-12T21:16:02Z

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (41 lines, 6 files), I've queued these reviewers:

Correctness — logic errors, race conditions, lifecycle issues
Security — vulnerabilities, secret exposure, injection
API Compatibility — breaking changes, release tags, type design
Performance — algorithmic regressions, memory leaks
Testing — coverage gaps, hollow tests

How this works

Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.
Tick Start review below to dispatch the review fleet.
After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.
This comment updates as new commits land; your reviewer selections are preserved.
Start review

Copilot

Pull request overview

Removes the deprecated ILoaderOptions.enableOfflineLoad API surface and the legacy Fluid.Container.enableOfflineLoad config read, consolidating offline-load control onto Fluid.Container.enableOfflineFull while preserving the default “enabled for interactive clients” behavior. Adds a defensive UsageError to prevent loading from pendingLocalState when offline-load is explicitly disabled.

Changes:

Remove enableOfflineLoad from ILoaderOptions and update the corresponding legacy beta API report.
Update container offline-load enablement to be driven solely by Fluid.Container.enableOfflineFull, and throw on pendingLocalState + explicit disable.
Remove enableOfflineLoad from the test-service-load options matrix and adjust offline stashing logic accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/test/test-service-load/src/runner.ts	Removes dependency on deprecated loader option when deciding whether to stash pending local state.
packages/test/test-service-load/src/optionsMatrix.ts	Drops deprecated `enableOfflineLoad` from generated loader option permutations.
packages/loader/container-loader/src/container.ts	Consolidates offline-load enablement onto `enableOfflineFull` and adds a defensive `UsageError` on misconfiguration.
packages/common/container-definitions/src/loader.ts	Removes deprecated `enableOfflineLoad` from `ILoaderOptions`.
packages/common/container-definitions/api-report/container-definitions.legacy.beta.api.md	Updates legacy beta API report to reflect removal of `enableOfflineLoad`.
.changeset/remove-enable-offline-load.md	Adds changeset documenting the breaking removal and migration guidance.

-				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.


anthony-murphy · 2026-05-12T21:19:42Z

 export type ILoaderOptions = {
    cache?: boolean;
    client?: IClient;
-    enableOfflineLoad?: boolean;


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

anthony-murphy · 2026-05-12T21:22:12Z

-				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) {


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

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.

anthony-murphy · 2026-05-12T21:33:51Z

Deep Review

Reviewed commit c61f1ef on 2026-05-12.

Readiness: 5/10 — MAKING PROGRESS

Not ready for sign-off. Two concerns flagged inline: a silent runtime semantic flip for consumers using the legacy Fluid.Container.enableOfflineLoad config-provider key (the new UsageError covers only the new key), and the Beta-Break-Process .N0 release-window / staging check you flagged in the PR description. Both have local fixes.

Path to Ready

Resolve inline threads
If Beta-Break-Process staging is required for @fluidframework/container-definitions at the current version, add the packages/test/breaks/client/X.Y0/ entry in this PR

Context for Reviewers

This completes the long-promised removal flagged in Remove dictionary from ILoaderOptions #19306 (ChumpChief, 2024-02-02), where anthony-murphy blocked unilateral removal with "we need a transition path". Remove dictionary from ILoaderOptions #19306 cleaned the typed-API footgun and deprecated the field; this PR is the runtime cleanup ~27 months later. The transition-path obligation now applies to the equally-public config-provider key, not just the typed surface — that is what drives S1 below.
Suggested reviewers based on blame/history: ChumpChief (owner of the Remove dictionary from ILoaderOptions #19306 typed-surface cleanup; api-report and Beta-Break-Process), anthony-murphy (cross-cutting; cited on Remove dictionary from ILoaderOptions #19306, stress test: add stashed ops #14502, Encapsulate container state attributes #19697), wes-carlson (stress-test offline scheduling: stress test: add scheduleOffline() #12214, stress test: add stashed ops #14502, offline: fix redirect table blob not being saved and re-enable stashed ops in stress test #16031), tyler-cai-microsoft ([Data Virtualization & Offline]: Reimplement groupId offline with the loader #20565, pending-state + snapshot-refresh interaction).

For human reviewer

Needs human judgment — release-window confirmation — Is this PR landing in a .N0 minor for @fluidframework/container-definitions, and is test/breaks/client/X.Y0/ staging mandatory at the current version? You explicitly invited this confirmation in the PR description; the pipeline cannot answer it.
Needs human judgment — design call on the new UsageError guard — You wrote it "happy to drop or relax it if reviewers prefer." Whether the guard should be permanent, temporary, or relaxed is a taste call for the area owners (ChumpChief / anthony-murphy).
Needs human judgment — strict vs conservative S1 fix — One reviewer-proposal direction would keep both config keys live with precedence for one release rather than only logging/throwing. Worth a call on whether the partner-migration risk warrants the conservative path for one more release.

anthony-murphy · 2026-05-12T21:33:52Z

+				"Cannot load from pending local state when Fluid.Container.enableOfflineFull is explicitly disabled",
+			);
+		}
+		const offlineLoadEnabled = this.isInteractiveClient && (explicitOfflineFull ?? true);


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.

anthony-murphy · 2026-05-12T21:33:53Z

-	enableOfflineLoad?: boolean;
-
 	/**
 	 * Provide the current Loader through the scope object when creating Containers. It is added


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).

Copilot AI review requested due to automatic review settings May 12, 2026 21:15

dannimad requested review from a team as code owners May 12, 2026 21:15

Copilot started reviewing on behalf of dannimad May 12, 2026 21:16 View session

dannimad added the deep-review label May 12, 2026

Copilot AI reviewed May 12, 2026

View reviewed changes

anthony-murphy reviewed May 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecated ILoaderOptions.enableOfflineLoad#27290

Remove deprecated ILoaderOptions.enableOfflineLoad#27290
dannimad wants to merge 1 commit into
microsoft:mainfrom
dannimad:remove-enableOfflineLoad-flag

dannimad commented May 12, 2026

Uh oh!

github-actions Bot commented May 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

anthony-murphy May 12, 2026

Uh oh!

anthony-murphy May 12, 2026

Uh oh!

dannimad May 12, 2026

Uh oh!

anthony-murphy commented May 12, 2026

Uh oh!

anthony-murphy May 12, 2026

Uh oh!

anthony-murphy May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dannimad commented May 12, 2026

Description

Breaking Changes

Reviewer Guidance

Uh oh!

github-actions Bot commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

anthony-murphy May 12, 2026

Choose a reason for hiding this comment

Uh oh!

anthony-murphy May 12, 2026

Choose a reason for hiding this comment

Uh oh!

dannimad May 12, 2026

Choose a reason for hiding this comment

Uh oh!

anthony-murphy commented May 12, 2026

Deep Review

Path to Ready

Context for Reviewers

Uh oh!

anthony-murphy May 12, 2026

Choose a reason for hiding this comment

Uh oh!

anthony-murphy May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants