feat: Add FDv2 data system support to NodeClient - #1775
Conversation
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@cursor 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 14bd7e4. Configure here.
65b044a to
c4aa363
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f4a4c39 to
bc6706f
Compare
fix: Restore FDv2 connection mode when an offline transition fails fix: Await in-flight FDv2 transition on duplicate-mode setConnectionMode
bc6706f to
254d1a2
Compare
| streaming/fdv2/reconnection state management/updates previously known state | ||
| streaming/fdv2/can discard partial events on errors | ||
|
|
||
| # Looks like an issue in the test harness where client SDKs are assumed to have a |
There was a problem hiding this comment.
Not fond of the wording of this.
There was a problem hiding this comment.
Are comments actually supported? I just assume they don't match any test names?
There was a problem hiding this comment.
I think I specified the wrong set of capabilities, this should have mobile capability as well. Since mobile sdks do not need to do the initial one-shot.
There was a problem hiding this comment.
okay, looking at this more, I think we will need to put in a fix in the test harness for client sdks (tracked in a different ticket). I let claude take a stab at the explanation.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
2 participants
SDK-2468
Note
Medium Risk
Touches core flag data plumbing and connection/offline semantics, but FDv1 behavior is preserved behind the existing data manager when
dataSystemis absent.Overview
When
dataSystemis configured, NodeClient now wirescreateFDv2DataManagerBase(desktop defaults, transition table, query params for client-side ID vs mobile) instead ofNodeDataManager, while leaving the FDv1 path unchanged.Connection mode is tracked on the client (
_connectionMode). In FDv2,setConnectionModevalidates modes, no-ops after close or on duplicate mode, serializes concurrent transitions (flush before offline, then event sending + data manager), andgetConnectionMode/isOfflineread that state rather than the FDv1 data manager.CI runs FDv2 contract tests (harness v3) against the same long-lived test service (
stop_service: 'false'), with a new suppressions file and harness updates (mobile,useMobileKey, extra capabilities). Unit coverage is added inNodeClientFDv2.test.ts;FDv2ConnectionModeis exported fromLDCommon.Reviewed by Cursor Bugbot for commit 01ac80d. Bugbot is set up for automated code reviews on this repo. Configure here.