fix: Implement anonymous context processing #350

yusinto · 2024-01-18T20:00:28Z

Apologies, I missed implementing this when moving the project to js-core.

shortcut-integration · 2024-01-18T20:00:32Z

This pull request has been linked to Shortcut Story #229700: Implement anonymous context processing.

yusinto · 2024-01-18T20:02:38Z

packages/shared/sdk-client/src/utils/calculateFlagChanges.ts

This was in index.ts but I moved it now to its own explicit file.

yusinto · 2024-01-18T20:06:22Z

packages/shared/sdk-client/src/utils/ensureKey.ts

+} from '@launchdarkly/js-sdk-common';
+import { isLegacyUser, isMultiKind, isSingleKind } from '@launchdarkly/js-sdk-common';
+
+export const ns = (s: string) => `LaunchDarkly_AnonKeys_${s}`;


I'm using LaunchDarkly_AnonKeys_ which is different from the existing js common. We may have to worry about migrating existing keys. Although for the react native sdk, since it's a brand new sdk using a new storage package, migration maybe difficult if not impossible.

Could we name this function namespaced or something more explicit. I read the later code before this and was like what is an nsKind?

yusinto · 2024-01-18T20:08:59Z

packages/shared/sdk-client/src/LDClientImpl.test.ts

@@ -133,6 +135,20 @@ describe('sdk-client object', () => {
    });
  });

+  test('identify anonymous', async () => {
+    defaultPutResponse['dev-test-flag'].value = false;
+    const carContext: LDContext = { kind: 'car', anonymous: true, key: '' };


Even anonymous, key is a required field so it has to be set to an empty string. I think this is fine, but calling this out for concerns/discussion.

If you are ok with that requirement. I was originally planning on exporting a different type from client SDKs that did not require a key. Then internal to the client implementation decorate it (generate keys, verify anonymous is set) and create a Context.

Because I think that the pattern we had before, where you don't include a key and that makes the context anonymous is very intuitive.

I'm fine with this for now.

kinyoklion · 2024-01-18T21:19:23Z

packages/shared/common/src/Context.ts

@@ -43,7 +43,7 @@ function encodeKey(key: string): string {
 * @param context
 * @returns true if the context is a single kind context.
 */
-function isSingleKind(context: LDContext): context is LDSingleKindContext {
+export function isSingleKind(context: LDContext): context is LDSingleKindContext {


I have serious reservations about exporting these functions. We don't want them exported from the final package and becoming part of the stable API of all of the packages.

Will you be open to exporting these from internal please? They are useful internally and the common sdks can re-use them for internal logic.

I am ok with them being in internal.

I am really concerned about them being useful outside these two cases. They really are not supposed to be.

We are provided an LDContext and we then validate that into a Context. Once we have a Context these functions are not useful.

So, if we are careful to try not to use LDContext, aside from adding keys to it and then constructing a context, then that is probably fine.

Agreed. I'll add comments in this file reflecting what you said.

…ocessing

kinyoklion · 2024-01-19T16:43:19Z

packages/shared/sdk-client/src/utils/ensureKey.ts

+  await ensureKeyCommon('user', c, platform);
+};
+
+const ensureKey = async (c: LDContext, platform: Platform) => {


I think I missed that we are modifying the context which is provided, versus modifying a copy. Typically we would never directly change things which are provided to the SDK.

We could either always clone, which is probably fine because identify isn't a high frequency situation.

Or only clone when we determine that we need to.

Ok I'll clone and return the new object from ensureKey.

🤖 I have created a release *beep* *boop* --- <details><summary>akamai-edgeworker-sdk-common: 1.1.0</summary> ## [1.1.0](akamai-edgeworker-sdk-common-v1.0.6...akamai-edgeworker-sdk-common-v1.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from ^2.1.3 to ^2.2.0 </details> <details><summary>akamai-server-base-sdk: 2.1.0</summary> ## [2.1.0](akamai-server-base-sdk-v2.0.6...akamai-server-base-sdk-v2.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.0.6 to ^1.1.0 * @launchdarkly/js-server-sdk-common bumped from ^2.1.3 to ^2.2.0 </details> <details><summary>akamai-server-edgekv-sdk: 1.1.0</summary> ## [1.1.0](akamai-server-edgekv-sdk-v1.0.14...akamai-server-edgekv-sdk-v1.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.0.6 to ^1.1.0 * @launchdarkly/js-server-sdk-common bumped from ^2.1.3 to ^2.2.0 </details> <details><summary>cloudflare-server-sdk: 2.4.0</summary> ## [2.4.0](cloudflare-server-sdk-v2.3.3...cloudflare-server-sdk-v2.4.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.1.3 to 2.2.0 </details> <details><summary>js-client-sdk-common: 0.2.0</summary> ## [0.2.0](js-client-sdk-common-v0.1.2...js-client-sdk-common-v0.2.0) (2024-02-06) ### Features * Implement common client side support for auto environment attributes. ([#356](#356)) ([8d80259](8d80259)) * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Bug Fixes * Add LDOptions.application name and versionName. ([#358](#358)) ([cd75210](cd75210)) * Add RN SDK offline support through ConnectionMode. ([#361](#361)) ([d97ce82](d97ce82)) * Implement anonymous context processing ([#350](#350)) ([308100d](308100d)) * Improvements and fixes from docs review. ([#362](#362)) ([ba07fbf](ba07fbf)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-sdk-common bumped from 2.1.1 to 2.2.0 </details> <details><summary>js-sdk-common: 2.2.0</summary> ## [2.2.0](js-sdk-common-v2.1.1...js-sdk-common-v2.2.0) (2024-02-06) ### Features * Implement common client side support for auto environment attributes. ([#356](#356)) ([8d80259](8d80259)) * Implement common support for auto environment attributes. ([#355](#355)) ([9f562e5](9f562e5)) * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Bug Fixes * Add LDOptions.application name and versionName. ([#358](#358)) ([cd75210](cd75210)) * Add RN SDK offline support through ConnectionMode. ([#361](#361)) ([d97ce82](d97ce82)) * Implement anonymous context processing ([#350](#350)) ([308100d](308100d)) * RN streamer connection in background and foreground. ([#360](#360)) ([c69b768](c69b768)) </details> <details><summary>js-server-sdk-common: 2.2.0</summary> ## [2.2.0](js-server-sdk-common-v2.1.3...js-server-sdk-common-v2.2.0) (2024-02-06) ### Features * Implement common client side support for auto environment attributes. ([#356](#356)) ([8d80259](8d80259)) * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Bug Fixes * Add LDOptions.application name and versionName. ([#358](#358)) ([cd75210](cd75210)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-sdk-common bumped from 2.1.1 to 2.2.0 </details> <details><summary>js-server-sdk-common-edge: 2.2.0</summary> ## [2.2.0](js-server-sdk-common-edge-v2.1.3...js-server-sdk-common-edge-v2.2.0) (2024-02-06) ### Features * Implement common client side support for auto environment attributes. ([#356](#356)) ([8d80259](8d80259)) * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.1.3 to 2.2.0 </details> <details><summary>node-server-sdk: 9.1.0</summary> ## [9.1.0](node-server-sdk-v9.0.6...node-server-sdk-v9.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.1.3 to 2.2.0 </details> <details><summary>node-server-sdk-dynamodb: 6.1.0</summary> ## [6.1.0](node-server-sdk-dynamodb-v6.0.6...node-server-sdk-dynamodb-v6.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.0.6 to 9.1.0 </details> <details><summary>node-server-sdk-redis: 4.1.0</summary> ## [4.1.0](node-server-sdk-redis-v4.0.6...node-server-sdk-redis-v4.1.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.0.6 to 9.1.0 </details> <details><summary>react-native-client-sdk: 0.2.0</summary> ## [0.2.0](react-native-client-sdk-v0.1.5...react-native-client-sdk-v0.2.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Bug Fixes * Add RN SDK offline support through ConnectionMode. ([#361](#361)) ([d97ce82](d97ce82)) * Implement RN SDK EventSource jitter backoff. ([#359](#359)) ([95e58bd](95e58bd)) * Improvements and fixes from docs review. ([#362](#362)) ([ba07fbf](ba07fbf)) * RN streamer connection in background and foreground. ([#360](#360)) ([c69b768](c69b768)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-client-sdk-common bumped from 0.1.2 to 0.2.0 </details> <details><summary>vercel-server-sdk: 1.3.0</summary> ## [1.3.0](vercel-server-sdk-v1.2.3...vercel-server-sdk-v1.3.0) (2024-02-06) ### Features * React-native support for auto-env attributes. Only affects react-native package. ([deea99c](deea99c)) * Update eslint jest configuration and versions. ([deea99c](deea99c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.1.3 to 2.2.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>

yusinto added 5 commits January 17, 2024 14:55

chore: ensure keys exist for anonymous.

1db8b18

chore: move and rename files to improve organization.

c03a83d

chore: add unit tests for ensureKey.

47b5a9d

chore: Added more ensureKey tests.

e2fda00

chore: add anonymous user test.

0826356

yusinto requested review from kinyoklion and louis-launchdarkly as code owners January 18, 2024 20:00

yusinto commented Jan 18, 2024

View reviewed changes

yusinto requested a review from tanderson-ld January 18, 2024 20:06

yusinto commented Jan 18, 2024

View reviewed changes

Update DiagnosticsManager.test.ts

a187dc1

kinyoklion reviewed Jan 18, 2024

View reviewed changes

yusinto added 3 commits January 18, 2024 15:31

chore: rename ns to addNamespace.

24c9ea3

chore: move kind helpers to internal.

cfdcb7f

Update index.ts

0835dbe

yusinto enabled auto-merge (squash) January 19, 2024 00:11

yusinto requested a review from kinyoklion January 19, 2024 00:11

Merge branch 'main' into yus/sc-229700/implement-anonymous-context-pr…

3224bdc

…ocessing

kinyoklion requested changes Jan 19, 2024

View reviewed changes

chore: Clone context in ensureKey to avoid mutating the original.

9daf1cc

yusinto requested a review from kinyoklion January 19, 2024 18:46

kinyoklion approved these changes Jan 19, 2024

View reviewed changes

yusinto merged commit 308100d into main Jan 19, 2024
16 checks passed

yusinto deleted the yus/sc-229700/implement-anonymous-context-processing branch January 19, 2024 18:54

github-actions bot mentioned this pull request Jan 19, 2024

chore: release main #353

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Implement anonymous context processing #350

fix: Implement anonymous context processing #350

yusinto commented Jan 18, 2024 •

edited

Loading

shortcut-integration bot commented Jan 18, 2024

yusinto Jan 18, 2024

yusinto Jan 18, 2024

kinyoklion Jan 18, 2024

yusinto Jan 18, 2024

kinyoklion Jan 18, 2024

yusinto Jan 19, 2024

kinyoklion Jan 18, 2024

yusinto Jan 18, 2024

kinyoklion Jan 18, 2024

yusinto Jan 18, 2024

kinyoklion Jan 19, 2024

yusinto Jan 19, 2024

yusinto Jan 19, 2024

fix: Implement anonymous context processing #350

fix: Implement anonymous context processing #350

Conversation

yusinto commented Jan 18, 2024 • edited Loading

shortcut-integration bot commented Jan 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yusinto commented Jan 18, 2024 •

edited

Loading