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 |
| 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.
| }; | ||
| 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?
There was a problem hiding this comment.
I think something of this sort will generally be required. Though they could be combined potentially into some "get all for kinds", but I don't see a huge amount of value in that.
With FDv2 they should at least always be in memory.
| } 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.
packages/shared/sdk-server/src/api/subsystems/LDFeatureStore.ts
Outdated
Show resolved
Hide resolved
| }; | ||
| doInit(oldData); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
I think something of this sort will generally be required. Though they could be combined potentially into some "get all for kinds", but I don't see a huge amount of value in that.
With FDv2 they should at least always be in memory.
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