launchdarkly / js-sdk-common

Code shared between all LaunchDarkly client-side JS-based SDKs
Other
3 stars 26 forks source link

Types could catch invalid contexts at build time #110

Open coracuity opened 3 weeks ago

coracuity commented 3 weeks ago

Describe the bug

The LDContextCommon type allows invalid contexts. The key field is required unless anonymous is set to true and this can be enforced at the type level. In the case key is not set and anonymous is undefined or false, the error may be missed during testing resulting in targeted features being unavailable.

This type would catch this issue for a multi-kind context:

interface LDMultiKindContext {
  kind: "multi";
  [kind: string]:
    | "multi"
    | (Omit<LDContextCommon, "key" | "anonymous"> & { key: string; anonymous?: false })
    | (Omit<LDContextCommon, "key" | "anonymous"> & { key?: string; anonymous: true });
}

To reproduce

In React:

const ldClient = useLDClient();
ldClient?.identify({ kind: "multi", user: { kind: "user", userId: "1234" } }, undefined, (error, flags) => {
  console.error("failed to initialize!", { error, flags });
});

Expected behavior

I would expect this to be caught by types.

Logs

N/A

SDK version

We're using js-sdk-common 5.2.0 at the moment, but the problem is present in js-sdk-common 5.3.0 as well.

Language version, developer tools

Node 20

OS/platform

macos, but this would happen on any platform.

Additional context

None

coracuity commented 1 week ago

Would this be something you'd accept a PR for?