chore: preparation work for FDv2 feature store and persistence support #725
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
tanderson-ld
changed the title
| this._featureStore.get(kind, key, doUpsert); | ||
| } else { | ||
| doUpsert(); | ||
| } |
There was a problem hiding this comment.
For reviewers: upsert and init have been combined and the distinction is now driven by the bool basis.
| [VersionedDataKinds.Features.namespace]: oldFlags, | ||
| [VersionedDataKinds.Segments.namespace]: oldSegments, | ||
| }; | ||
| doInit(oldData); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
For reviewers: These all calls feel a bit off, but without a refactoring of layer responsibility, I don't see a change that is worth the effort. Thoughts?
| } else { | ||
| items[key] = item; | ||
| } | ||
| } |
There was a problem hiding this comment.
For reviewers: This logic has moved into apply changes.
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
For reviewers: Guts of this logic comes from _addItem at the top of this file. Small tweaks from there.
| }); | ||
| _callback(); | ||
| } | ||
| } |
There was a problem hiding this comment.
For reviewers: This code is not shippable (why this PR is going to a temp branch) due to the intermediate states potentially being invalid, but this temporary code allows contract tests not checking intermediate state to pass.
Splitting these changes up into multiple PRs to keep them manageable and because we can't merge directly to main as
applyChangesis dependent on transaction support in the PersistentDataStore.Existing tests are exercising applyChanges, but through init, upsert, and delete. Plan to add applyChanges tests in ta/fdv2-temporary-holding after transaction support is added to PersistentDataStore in a subsequent PR as part of SDK-1029