feat: Add TTL caching for data store #801

keelerm84 · 2025-03-14T14:33:36Z

No description provided.

keelerm84 · 2025-03-14T14:34:42Z

@kinyoklion do you want me to split this up so we can control the version bumps and change log messages separately?

github-actions · 2025-03-14T14:35:02Z

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 19014 bytes
Size limit: 21000

github-actions · 2025-03-14T14:35:18Z

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 15371 bytes
Size limit: 20000

github-actions · 2025-03-14T14:35:22Z

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Size: 19871 bytes
Size limit: 21000

keelerm84 · 2025-03-14T14:53:13Z

packages/sdk/cloudflare/src/index.ts

+   * Optional TTL cache configuration which allows for caching feature flags in
+   * memory.
+   */
+  cache?: internalServer.TtlCacheOptions;


Do I need to re-export the TtlCacheOptions type? Or is that just automatically exposed since it's part of another public type?

It would need re-exported. It is technically available, but under internalServer. If you want people to use it, then it should just be exported at the package level.

Okay, I believe I have addressed this correctly.

keelerm84 · 2025-03-14T14:55:07Z

packages/shared/sdk-server-edge/src/api/createOptions.ts

 const createOptions = (options: LDOptions) => {
  const finalOptions = { ...defaultOptions, ...options };
-  finalOptions.logger?.debug(`Using LD options: ${JSON.stringify(finalOptions)}`);
+  // finalOptions.logger?.debug(`Using LD options: ${JSON.stringify(finalOptions)}`);


I cannot leave this in. When I do and try to run the unit tests, I get

TypeError: Converting circular structure to JSON --> starting at object with constructor 'Timeout' | property '_idlePrev' -> object with constructor 'TimersList' --- property '_idleNext' closes the circle at JSON.stringify (<anonymous>) 11 | const createOptions = (options: LDOptions) => { 12 | const finalOptions = { ...defaultOptions, ...options }; > 13 | finalOptions.logger?.debug(`Using LD options: ${JSON.stringify(finalOptions)}`); | ^ 14 | return finalOptions; 15 | }; 16 | at createOptions (../../shared/sdk-server-edge/src/api/createOptions.ts:13:56) at new LDClient (../../shared/sdk-server-edge/src/api/LDClient.ts:25:39) at init (../../shared/sdk-server-edge/src/index.ts:26:10) at init (src/index.ts:69:18) at Object.<anonymous> (__tests__/index.test.ts:158:28)

Could use some help figuring that bit out.

keelerm84 · 2025-03-14T14:56:12Z

packages/shared/sdk-server/src/cache/TtlCache.ts

 /**
 * Options for the TTL cache.
- *
- * @internal


@kinyoklion you had mentioned wanting to add some documentation in this PR to talk about not using certain types. Was that for these classes specifically since we are removing this tag, or was this a broader thing you wanted added in like CONTRIBUTING or something?

I may have been talking about adding some contributing docs. I started adding a bit of a developers guide to the contributing doc.

Basically I want to prevent a recurrence of the various internal types being exported in leaf-node packages. Once we can fix that, then we can get rid of this internal namespace ideal all together.

I also think we need to add some readme files to non-leaf packages similar to the ones we put in flutter.

https://github.com/launchdarkly/flutter-client-sdk/tree/main/packages/common_client#readme

Not blocking this PR though.

kinyoklion · 2025-03-14T17:46:24Z

@kinyoklion do you want me to split this up so we can control the version bumps and change log messages separately?

I think that one to expose the caching from common and another to use it would be more ideal.

That said I would be ok with these being combined, but the release note would need to be reasonable for the combined version.

keelerm84 · 2025-03-14T20:42:24Z

@kinyoklion do you want me to split this up so we can control the version bumps and change log messages separately?

I think that one to expose the caching from common and another to use it would be more ideal.

That said I would be ok with these being combined, but the release note would need to be reasonable for the combined version.

https://github.com/launchdarkly/js-core/pull/804/files for the first bit.

keelerm84 · 2025-03-17T17:08:13Z

packages/sdk/cloudflare/__tests__/index.test.ts

-  afterAll(() => {
-    ldClient.close();
-  });
+  describe('without caching', () => {


I recommend hiding whitespace.

kinyoklion · 2025-03-17T19:49:51Z

packages/sdk/cloudflare/__tests__/index.test.ts

-      const value = await ldClient.variation(flagKey3, contextWithCountry, false);
-      const detail = await ldClient.variationDetail(flagKey3, contextWithCountry, false);
+  describe('with caching', () => {
+    test('will cache across multiple variation calls', async () => {


Please use it instead of test. I know this file may have test, but I am trying to get everything to consistently be it from now on.

packages/shared/sdk-server-edge/src/api/cache.ts

kinyoklion · 2025-03-17T19:52:49Z

packages/shared/sdk-server-edge/src/api/EdgeFeatureStore.ts

+   * This method is used to retrieve the environment payload from the edge
+   * provider. If a cache is provided, it will serve from that.
+   *
+   * @returns


Could wither remove the @returns or fill the doc for it.

packages/shared/sdk-server-edge/src/api/EdgeFeatureStore.ts

🤖 I have created a release *beep* *boop* --- <details><summary>akamai-edgeworker-sdk-common: 2.0.1</summary> ## [2.0.1](akamai-edgeworker-sdk-common-v2.0.0...akamai-edgeworker-sdk-common-v2.0.1) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from ^2.11.1 to ^2.12.0 </details> <details><summary>akamai-server-base-sdk: 3.0.1</summary> ## [3.0.1](akamai-server-base-sdk-v3.0.0...akamai-server-base-sdk-v3.0.1) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^2.0.0 to ^2.0.1 * @launchdarkly/js-server-sdk-common bumped from ^2.11.1 to ^2.12.0 </details> <details><summary>akamai-server-edgekv-sdk: 1.4.3</summary> ## [1.4.3](akamai-server-edgekv-sdk-v1.4.2...akamai-server-edgekv-sdk-v1.4.3) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^2.0.0 to ^2.0.1 * @launchdarkly/js-server-sdk-common bumped from ^2.11.1 to ^2.12.0 </details> <details><summary>cloudflare-server-sdk: 2.7.0</summary> ## [2.7.0](cloudflare-server-sdk-v2.6.5...cloudflare-server-sdk-v2.7.0) (2025-03-17) ### Features * Add TTL caching for data store ([#801](#801)) ([c1de485](c1de485)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.5.4 to 2.6.0 </details> <details><summary>fastly-server-sdk: 0.1.1</summary> ## [0.1.1](fastly-server-sdk-v0.1.0...fastly-server-sdk-v0.1.1) (2025-03-17) ### Bug Fixes * Remove logging of SDK option configurations ([#806](#806)) ([a76d196](a76d196)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.11.1 to 2.12.0 </details> <details><summary>js-server-sdk-common: 2.12.0</summary> ## [2.12.0](js-server-sdk-common-v2.11.1...js-server-sdk-common-v2.12.0) (2025-03-17) ### Features * Export internalServer module for internal LD usage ([#804](#804)) ([ec43ac8](ec43ac8)) </details> <details><summary>js-server-sdk-common-edge: 2.6.0</summary> ## [2.6.0](js-server-sdk-common-edge-v2.5.4...js-server-sdk-common-edge-v2.6.0) (2025-03-17) ### Features * Add TTL caching for data store ([#801](#801)) ([c1de485](c1de485)) ### Bug Fixes * Remove logging of SDK option configurations ([#806](#806)) ([a76d196](a76d196)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.11.1 to 2.12.0 </details> <details><summary>node-server-sdk: 9.7.5</summary> ## [9.7.5](node-server-sdk-v9.7.4...node-server-sdk-v9.7.5) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.11.1 to 2.12.0 </details> <details><summary>node-server-sdk-dynamodb: 6.2.5</summary> ## [6.2.5](node-server-sdk-dynamodb-v6.2.4...node-server-sdk-dynamodb-v6.2.5) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.7.4 to 9.7.5 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.7.5 </details> <details><summary>node-server-sdk-otel: 1.1.5</summary> ## [1.1.5](node-server-sdk-otel-v1.1.4...node-server-sdk-otel-v1.1.5) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.7.4 to 9.7.5 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.7.5 </details> <details><summary>node-server-sdk-redis: 4.2.5</summary> ## [4.2.5](node-server-sdk-redis-v4.2.4...node-server-sdk-redis-v4.2.5) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.7.4 to 9.7.5 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.7.5 </details> <details><summary>server-sdk-ai: 0.9.2</summary> ## [0.9.2](server-sdk-ai-v0.9.1...server-sdk-ai-v0.9.2) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/js-server-sdk-common bumped from 2.11.1 to 2.12.0 * peerDependencies * @launchdarkly/js-server-sdk-common bumped from 2.x to 2.12.0 </details> <details><summary>vercel-server-sdk: 1.3.24</summary> ## [1.3.24](vercel-server-sdk-v1.3.23...vercel-server-sdk-v1.3.24) (2025-03-17) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.5.4 to 2.6.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

keelerm84 requested a review from a team as a code owner March 14, 2025 14:33

keelerm84 commented Mar 14, 2025

View reviewed changes

keelerm84 force-pushed the mk/sdk-1121/caching branch from 0fd6dc3 to b686318 Compare March 14, 2025 20:50

keelerm84 changed the title ~~feat: Add TTL caching support~~ feat: Add TTL caching for data store Mar 14, 2025

keelerm84 force-pushed the mk/sdk-1121/caching branch from b686318 to c39a648 Compare March 17, 2025 17:07

keelerm84 commented Mar 17, 2025

View reviewed changes

feat: Add TTL caching support

45602db

keelerm84 force-pushed the mk/sdk-1121/caching branch from c39a648 to 45602db Compare March 17, 2025 18:51

Export cache options

2578e67

keelerm84 requested a review from kinyoklion March 17, 2025 19:00

kinyoklion reviewed Mar 17, 2025

View reviewed changes

test -> it

7707986

kinyoklion reviewed Mar 17, 2025

View reviewed changes

packages/shared/sdk-server-edge/src/api/cache.ts Show resolved Hide resolved

kinyoklion reviewed Mar 17, 2025

View reviewed changes

keelerm84 added 2 commits March 17, 2025 15:54

Add doc

1811022

Remove redundant return

6d010c0

keelerm84 force-pushed the mk/sdk-1121/caching branch from 408e811 to 6d010c0 Compare March 17, 2025 20:17

kinyoklion reviewed Mar 17, 2025

View reviewed changes

packages/shared/sdk-server-edge/src/api/EdgeFeatureStore.ts Outdated Show resolved Hide resolved

Tweak error message

893ab93

keelerm84 force-pushed the mk/sdk-1121/caching branch from 73ff0cb to 14ca6cd Compare March 17, 2025 20:34

Don't allow undefined

255bb04

keelerm84 force-pushed the mk/sdk-1121/caching branch from 14ca6cd to 255bb04 Compare March 17, 2025 20:38

kinyoklion approved these changes Mar 17, 2025

View reviewed changes

keelerm84 merged commit c1de485 into main Mar 17, 2025
23 checks passed

keelerm84 deleted the mk/sdk-1121/caching branch March 17, 2025 20:50

github-actions bot mentioned this pull request Mar 17, 2025

chore: release main #805

Merged

feat: Add TTL caching for data store #801

feat: Add TTL caching for data store #801

Uh oh!

Conversation

keelerm84 commented Mar 14, 2025

Uh oh!

keelerm84 commented Mar 14, 2025

Uh oh!

github-actions bot commented Mar 14, 2025

Uh oh!

github-actions bot commented Mar 14, 2025

Uh oh!

github-actions bot commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinyoklion commented Mar 14, 2025

Uh oh!

keelerm84 commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants