feat: Add FDv2 DataManager and enable experimental FDv2 support.#1161
feat: Add FDv2 DataManager and enable experimental FDv2 support.#1161kinyoklion wants to merge 27 commits intomainfrom
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
packages/sdk/browser/src/options.ts
Outdated
| * the new `/sdk/poll/eval` and `/sdk/stream/eval` endpoints with the | ||
| * FDv2 protocol handler and orchestrator. | ||
| * | ||
| * Default: false |
There was a problem hiding this comment.
| * Default: false | |
| * @hidden | |
| * Default: false |
kinyoklion
force-pushed
the
mk/NOTICKET/testing-fdv2
branch
2 times, most recently
from
5e2b2cb to
4f882ca
Compare
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
force-pushed
the
mk/NOTICKET/testing-fdv2
branch
from
4f882ca to
a7413c0
Compare
|
@launchdarkly/js-client-sdk-common size report |
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; | ||
|
|
There was a problem hiding this comment.
This may change when we determine how we are going to handle setConnectionMode.
kinyoklion
changed the title
There was a problem hiding this comment.
For now all the size limits are 10% over the size of the packages in this PR.
After we have everything assembled we can do another pass to decrease size. Most of the size changes probably won't be related to the changes in this PR itself.
We already know that we have some barrel related problems with internal and it is time to fix the server-side exports.
There was a problem hiding this comment.
I think, with a few tweaks, we can maybe eliminate this entirely.
There was a problem hiding this comment.
We will want to remove this before we commit.
| readonly endpoints?: EndpointConfig; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This allows us to exclude cache from being a potential synchronizer.
| }; | ||
|
|
||
| const dataSourceEntryArrayValidator = arrayOf('type', { | ||
| const initializerEntryArrayValidator = arrayOf('type', { |
There was a problem hiding this comment.
Extend the typing changes to the runtime validation.
There was a problem hiding this comment.
I feel like there is two much code inside this.
There was a problem hiding this comment.
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.
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // @ts-ignore dataSystem is @internal — experimental FDv2 opt-in | ||
| dataSystem: {}, | ||
| logger: basicLogger({ level: 'debug' }), | ||
| }); |
There was a problem hiding this comment.
Example app uses experimental internal API with debug logging
Low Severity
The example app was modified to opt into the experimental FDv2 data system via dataSystem: {} (an @internal API requiring @ts-ignore) and to enable debug-level logging with basicLogger({ level: 'debug' }). This changes the public-facing example from standard SDK usage to experimental behavior that isn't ready for consumers. A PR reviewer noted code that needs to be removed before committing.
| 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')) |
There was a problem hiding this comment.
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.`, |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
|
bugbot review |
2 participants
Connects together most of the FDv2 components we have with an FDv2 DataManager. The configuration is
@internaland 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
DataManagerimplementation (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.dataSystemis present: it builds FDv2 query params, selects FDv2 polling endpoints, wires listener-driven streaming via optionalDataManagerhooks, and triggers immediate event flushing on background transitions.Tightens FDv2 mode-table typing/validation by separating
initializersvssynchronizers(disallowingcacheas 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.