feat: support per-mode FDv1 fallback configuration#1246
Conversation
The FDv1 fallback polling interval and endpoints are now configurable per connection mode instead of using a single global pollInterval. Background mode defaults to 1 hour, foreground modes default to 5 minutes. When a user overrides a mode without specifying fdv1Fallback, the built-in default is preserved.
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
|
bugbot review |
A partial fdv1Fallback override (e.g. only endpoints) no longer discards the mode's default pollInterval. The merge now spreads the default fallback config under the user override at the field level.
|
bugbot review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 118f816. Configure here.
| setFlushCallback(callback: () => void): void; | ||
| } | ||
|
|
||
| function mergeModeTables( |
There was a problem hiding this comment.
Wouldn't the validator already handle most of what this function is doing?
There was a problem hiding this comment.
Not as it is, but I am looking into it.
There was already a mode merge for modes that weren't defined.
There are defaults for incorrect definitions.
But not validation time defaults exactly.
|
going to approve since the potential validator redundancy comment should be a nit imo |
Wire MODE_TABLE as the default for connectionModes in the dataSystem validator so per-mode fdv1Fallback defaults (pollInterval, endpoints) are filled in during validation. This removes the mergeModeTables function and restores the simple spread merge in FDv2DataManagerBase. Also adds a defaults option to recordOf (matching validatorOf pattern) for compound validators that need built-in defaults.
Add direct unit tests for the recordOf built-in defaults option in validateOptions.test.ts. Add integration tests in LDClientDataSystemOptions.test.ts verifying fdv1Fallback defaults flow through the full dataSystem validation pipeline.
Remove the FDv2DataManagerBase test that hardcoded validated output instead of testing the actual validation path. Add comprehensive fdv1Fallback permutation tests to the dataSystem validation suite covering: interval only, endpoints only, both, empty object, non-number pollInterval, non-string endpoint URIs, and non-object fdv1Fallback. Each verifies that defaults are filled in and invalid values are dropped with warnings.
The FDv1 fallback polling interval and endpoints are now configurable per connection mode instead of using a single global pollInterval. Background mode defaults to 1 hour, foreground modes default to 5 minutes. When a user overrides a mode without specifying fdv1Fallback, the built-in default is preserved.
Note
Medium Risk
Moderate risk because it changes how the FDv1 fallback synchronizer is constructed (poll interval and service endpoints) based on per-mode configuration, which could affect data sync behavior across modes. Changes are scoped and covered by new validation and data manager tests.
Overview
Adds per-connection-mode configuration for the FDv1 polling fallback via a new
ModeDefinition.fdv1Fallback(poll interval + endpoint overrides) and exportsFDv1FallbackConfig.Updates the built-in
MODE_TABLEto include FDv1 fallback defaults (5 min for foreground modes, 1 hr forbackground) and extends validation (modeDefinitionValidators,recordOfdefaults precedence, and top-leveldataSystemdefaults) so partial mode overrides preserve these built-in fallback defaults.Changes
FDv2DataManagerBaseto build the FDv1 fallback synchronizer using the mode’sfdv1Fallbackpoll interval and optional per-modeServiceEndpoints, and adds tests covering defaulting/merging and correct poll interval selection for background mode and endpoint-only overrides.Reviewed by Cursor Bugbot for commit 749c59f. Bugbot is set up for automated code reviews on this repo. Configure here.