Skip to content

feat: Add FDv2 DataManager and enable experimental FDv2 support.#1161

Closed
kinyoklion wants to merge 40 commits intomainfrom
mk/NOTICKET/testing-fdv2
Closed

feat: Add FDv2 DataManager and enable experimental FDv2 support.#1161
kinyoklion wants to merge 40 commits intomainfrom
mk/NOTICKET/testing-fdv2

Conversation

@kinyoklion
Copy link
Copy Markdown
Member

@kinyoklion kinyoklion commented Mar 6, 2026

Connects together most of the FDv2 components we have with an FDv2 DataManager. The configuration is @internal and will remain that way until we are ready for consumers.

Doesn't:
This doesn't yet address the public API changes for being able to directly set a connection mode.
It doesn't add support for custom modes.
It doesn't add the inputs to the automatic mode switching.
Contract tests (unless Todd adds them and this comment becomes irrelevant.

Does:
Connect together F2v2 E2E.
Adds the the data manager.
Includes the mode debouncing.


Note

High Risk
High risk because it introduces a new FDv2 data-management path (mode resolution, polling/streaming orchestration, FDv1 fallback) and conditionally switches the browser SDK to new endpoints based on dataSystem, which can impact flag delivery and streaming behavior.

Overview
Adds an FDv2 DataManager implementation (createFDv2DataManagerBase) that owns connection-mode resolution, debounced lifecycle/network transitions, FDv2 data source orchestration, and optional FDv1 polling fallback.

Updates the browser SDK to opt into FDv2 when configuration.dataSystem is present: it builds FDv2 query params, selects FDv2 polling endpoints, wires listener-driven streaming via optional DataManager hooks, and triggers immediate event flushing on background transitions.

Tightens FDv2 mode-table typing/validation by separating initializers vs synchronizers (disallowing cache as a synchronizer) and expands exports/tests accordingly; also tweaks FDv2 synchronizer fallback logging and updates examples/CI package size thresholds.

Written by Cursor Bugbot for commit 304e5f3. This will update automatically on new commits. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 25661 bytes
Compressed size limit: 29000
Uncompressed size: 126143 bytes

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

@launchdarkly/browser size report
This is the brotli compressed size of the ESM build.
Compressed size: 178822 bytes
Compressed size limit: 194000
Uncompressed size: 828026 bytes

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Compressed size: 31069 bytes
Compressed size limit: 34000
Uncompressed size: 110826 bytes

* the new `/sdk/poll/eval` and `/sdk/stream/eval` endpoints with the
* FDv2 protocol handler and orchestrator.
*
* Default: false
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
* Default: false
* @hidden
* Default: false

@kinyoklion kinyoklion force-pushed the mk/NOTICKET/testing-fdv2 branch 2 times, most recently from 5e2b2cb to 4f882ca Compare March 10, 2026 17:03
kinyoklion and others added 9 commits March 10, 2026 14:13
Add `dataSystem` option to LDOptions for FDv2 data system configuration.
The field is marked @internal and stripped from public type declarations
via stripInternal. When present, ConfigurationImpl deep-validates it
using dataSystemValidators with platform-specific defaults passed via
LDClientInternalOptions.dataSystemDefaults.

- Browser SDK passes BROWSER_DATA_SYSTEM_DEFAULTS
- React Native SDK passes MOBILE_DATA_SYSTEM_DEFAULTS
- No behavioral changes — the configuration is accepted and validated
  but not yet wired to any data manager selection logic

SDK-1935
Convert validators from a static constant to a createValidators()
factory function that accepts platform-specific dataSystemDefaults.
When defaults are provided, dataSystem uses validatorOf() with
built-in defaults for deep nested validation in a single pass.

Add builtInDefaults parameter to validatorOf() so the compound
validator carries its own defaults without relying on parent-level
defaults (which would leak into the result for absent fields).

Remove the separate deep-validation block from ConfigurationImpl —
the generic validateOptions call now handles everything.

Add tests for non-object dataSystem, granular automaticModeSwitching,
and no-leakage of dataSystem defaults to other config fields.
@kinyoklion kinyoklion force-pushed the mk/NOTICKET/testing-fdv2 branch from 4f882ca to a7413c0 Compare March 10, 2026 22:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 36171 bytes
Compressed size limit: 38000
Uncompressed size: 199479 bytes

kinyoklion and others added 9 commits March 11, 2026 13:11
Resolved conflicts in sdk-client index.ts (kept FDv2 orchestration
exports needed by integration branch) and BrowserClient.ts (kept
single-object super() call structure).
Resolved conflict in StreamingFDv2Base.ts: took main's stopped guard
and errorInfoFromNetworkError helper, removed unused DataSourceErrorKind
import.
* Closes the data manager. Any active connections are closed.
*/
close(): void;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This may change when we determine how we are going to handle setConnectionMode.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think, with a few tweaks, we can maybe eliminate this entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We will want to remove this before we commit.

readonly endpoints?: EndpointConfig;
}

/**
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This allows us to exclude cache from being a potential synchronizer.

};

const dataSourceEntryArrayValidator = arrayOf('type', {
const initializerEntryArrayValidator = arrayOf('type', {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Extend the typing changes to the runtime validation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I feel like there is two much code inside this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe the majority is just typing, so the volume isn't too bad, but this is a dependency hot-point. Lots of things coming together here.

@kinyoklion
Copy link
Copy Markdown
Member Author

bugbot review

modeDef.initializers
// Skip cache when bootstrapped — bootstrap data was applied to the
// flag store before identify, so the cache would only load older data.
.filter((entry) => !(bootstrapped && entry.type === 'cache'))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could generalize this, but it feels like a YAGNI situation.

initializerFactories.push(factory);
} else {
logger.warn(
`${logTag} Unsupported initializer type '${entry.type}'. It will be skipped.`,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The typing, for now, prevents this, but it will be possible with custom modes.

I wonder if it should be an error?

}
if (forcedStreaming === false) {
// Explicitly forced off — use configured mode, but never streaming.
return initialForegroundMode === 'streaming' ? 'one-shot' : initialForegroundMode;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is somewhat jank, but maintains the functionality from the existing implementation.

In the future we can remove setStreaming and things will be more logical with setConnectionMode.

@kinyoklion
Copy link
Copy Markdown
Member Author

bugbot review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

@kinyoklion kinyoklion closed this Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants