Types could catch invalid contexts at build time

[coracuity]

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]

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

[kinyoklion]

Hello @coracuity,

It appears this item was missed, sorry about that. The idea of the above improvement seems reasonable. I think it would be more clear with 2 types discriminated by the anonymous property (functionally the same, but just giving things names).

Unfortunately though I think the change would need to be a major version, as code that previously compiled and worked (with exception handling), would not subsequently compile with the updated types.

That said we are working on a new implementation, and will take this into consideration.

Thank you,
Ryan