feat: Correct client evaluation typings. #554

kinyoklion · 2024-08-27T23:01:17Z

Some code was refactored to make server and client share evaluation results. This should not have been done because the results are not the same in client and server SDKs. Server SDKs can always produce a reason and client SDKs cannot.

This meant that the typing said that reason was required in the client SDK, but it could be null.

This is a 'feat' to ensure a minor version bump in case of minor incompatibilities.

kinyoklion · 2024-08-27T23:01:46Z

packages/shared/common/src/api/data/LDEvaluationDetail.ts

@@ -1,6 +1,8 @@
 import { LDEvaluationReason } from './LDEvaluationReason';
 import { LDFlagValue } from './LDFlagValue';

+// TODO: On major version change "variationIndex" to only be optional and not nullable.


The variationIndex can be null because we maintained the 7.0 API when we released 8. We should change it to just be optional.

kinyoklion · 2024-08-27T23:02:21Z

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

@@ -483,7 +485,7 @@ export default class LDClientImpl implements LDClient {
    defaultValue: any,
    eventFactory: EventFactory,
    typeChecker?: (value: any) => [boolean, string],
-  ): LDFlagValue {
+  ): LDEvaluationDetail {


This type was wrong, basically it was any which means that the type checker could not reveal the problem.

kinyoklion · 2024-08-27T23:02:58Z

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

This file is not used outside of the common client code, and therefore is not part of the API.

kinyoklion · 2024-08-27T23:04:05Z

packages/shared/sdk-client/src/api/LDEvaluationDetail.ts

+// used by server SDKs, has a required reason. This file contains a client specific
+// LDEvaluationDetail which has an optional reason.
+
+// TODO: On major version change "reason" to be optional instead of nullable.


Reason should have been optional from day 1, but it was not.

Second it should not be null, but that is what the code was actually returning, so it will continue to do so.

The null fields were only for compatibility with the pre-typescript node API.

kinyoklion · 2024-08-27T23:04:33Z

packages/shared/sdk-client/src/api/LDInspection.ts

Inspectors were not implemented, and this is not in the interface, so remove it until we implement them. (Unless we only implement hooks.)

kinyoklion · 2024-08-27T23:04:57Z

packages/shared/sdk-client/src/configuration/validators.ts

Removed unimplemented fields.

kinyoklion · 2024-08-27T23:06:04Z

packages/shared/sdk-server/src/evaluation/EvalResult.ts

@@ -31,11 +30,22 @@ export default class EvalResult {
  }

  static forError(errorKind: internal.ErrorKinds, message?: string, def?: any): EvalResult {
-    return new EvalResult(true, createErrorEvaluationDetail(errorKind, def), message);
+    return new EvalResult(


Changed this code back to the way it was before the refactoring.

tanderson-ld · 2024-08-28T14:32:10Z

packages/shared/sdk-client/src/api/LDEvaluationDetail.ts

+ *
+ * This is the result of calling detailed variation methods.
+ */
+export type LDEvaluationDetailTyped<TFlag> = Omit<CommonDetailTyped<TFlag>, 'reason'> & {


Is the LDEvaluationDetailTyped as CommonDetailTyped required to make the type checking / compilation happy?

I am exporting a new type with the same name as the original type. So I need to import the original type under a different name, otherwise it will conflict.

tanderson-ld · 2024-08-28T14:33:06Z

packages/shared/sdk-client/src/api/LDInspection.ts

Not consumable publicly?

It was exported, but it wasn't in the options and it wasn't used.

So, another technically breaking change, but unrelated to any usable code.

We may not actually implement inspectors at all, as hooks have now replaced them.

🤖 I have created a release *beep* *boop* --- <details><summary>akamai-edgeworker-sdk-common: 1.1.13</summary> ## [1.1.13](akamai-edgeworker-sdk-common-v1.1.12...akamai-edgeworker-sdk-common-v1.1.13) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0 </details> <details><summary>akamai-server-base-sdk: 2.1.13</summary> ## [2.1.13](akamai-server-base-sdk-v2.1.12...akamai-server-base-sdk-v2.1.13) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.1.12 to ^1.1.13 * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0 </details> <details><summary>akamai-server-edgekv-sdk: 1.1.13</summary> ## [1.1.13](akamai-server-edgekv-sdk-v1.1.12...akamai-server-edgekv-sdk-v1.1.13) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/akamai-edgeworker-sdk-common bumped from ^1.1.12 to ^1.1.13 * @launchdarkly/js-server-sdk-common bumped from ^2.4.5 to ^2.5.0 </details> <details><summary>cloudflare-server-sdk: 2.5.11</summary> ## [2.5.11](cloudflare-server-sdk-v2.5.10...cloudflare-server-sdk-v2.5.11) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.3.6 to 2.3.7 </details> <details><summary>js-client-sdk-common: 1.6.0</summary> ## [1.6.0](js-client-sdk-common-v1.5.0...js-client-sdk-common-v1.6.0) (2024-08-28) ### Features * Correct client evaluation typings. ([#554](#554)) ([64ab88d](64ab88d)) * Make timeout optional in LDIdentifyOptions. ([#552](#552)) ([fa247b2](fa247b2)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-sdk-common bumped from 2.6.0 to 2.7.0 </details> <details><summary>js-sdk-common: 2.7.0</summary> ## [2.7.0](js-sdk-common-v2.6.0...js-sdk-common-v2.7.0) (2024-08-28) ### Features * Correct client evaluation typings. ([#554](#554)) ([64ab88d](64ab88d)) </details> <details><summary>js-server-sdk-common: 2.5.0</summary> ## [2.5.0](js-server-sdk-common-v2.4.5...js-server-sdk-common-v2.5.0) (2024-08-28) ### Features * Correct client evaluation typings. ([#554](#554)) ([64ab88d](64ab88d)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-sdk-common bumped from 2.6.0 to 2.7.0 </details> <details><summary>js-server-sdk-common-edge: 2.3.7</summary> ## [2.3.7](js-server-sdk-common-edge-v2.3.6...js-server-sdk-common-edge-v2.3.7) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.4.5 to 2.5.0 </details> <details><summary>node-server-sdk: 9.5.2</summary> ## [9.5.2](node-server-sdk-v9.5.1...node-server-sdk-v9.5.2) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common bumped from 2.4.5 to 2.5.0 </details> <details><summary>node-server-sdk-dynamodb: 6.1.19</summary> ## [6.1.19](node-server-sdk-dynamodb-v6.1.18...node-server-sdk-dynamodb-v6.1.19) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2 </details> <details><summary>node-server-sdk-otel: 1.0.11</summary> ## [1.0.11](node-server-sdk-otel-v1.0.10...node-server-sdk-otel-v1.0.11) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2 </details> <details><summary>node-server-sdk-redis: 4.1.19</summary> ## [4.1.19](node-server-sdk-redis-v4.1.18...node-server-sdk-redis-v4.1.19) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/node-server-sdk bumped from 9.5.1 to 9.5.2 * peerDependencies * @launchdarkly/node-server-sdk bumped from >=9.4.3 to >=9.5.2 </details> <details><summary>react-native-client-sdk: 10.6.0</summary> ## [10.6.0](react-native-client-sdk-v10.5.1...react-native-client-sdk-v10.6.0) (2024-08-28) ### Features * custom storage option for React Native SDK ([#539](#539)) ([115bd82](115bd82)) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-client-sdk-common bumped from 1.5.0 to 1.6.0 </details> <details><summary>vercel-server-sdk: 1.3.14</summary> ## [1.3.14](vercel-server-sdk-v1.3.13...vercel-server-sdk-v1.3.14) (2024-08-28) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-server-sdk-common-edge bumped from 2.3.6 to 2.3.7 </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>

feat: Correct client evaluation typings.

c6bfe2b

kinyoklion commented Aug 27, 2024

View reviewed changes

packages/shared/sdk-client/src/configuration/validators.ts Outdated

Copy link

Member Author

kinyoklion Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed unimplemented fields.

kinyoklion commented Aug 27, 2024

View reviewed changes

kinyoklion added 3 commits August 27, 2024 16:07

lint

0e68c8d

More cleanup.

54f5c11

Fix emitter import in test.

f8fc8eb

kinyoklion marked this pull request as ready for review August 27, 2024 23:13

kinyoklion requested a review from a team as a code owner August 27, 2024 23:13

tanderson-ld approved these changes Aug 28, 2024

View reviewed changes

kinyoklion merged commit 64ab88d into main Aug 28, 2024
20 checks passed

kinyoklion deleted the rlamb/sc-249895/fix-client-eval-typings branch August 28, 2024 16:12

github-actions bot mentioned this pull request Aug 28, 2024

chore: release main #553

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Correct client evaluation typings. #554

feat: Correct client evaluation typings. #554

kinyoklion commented Aug 27, 2024 •

edited

Loading

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

kinyoklion Aug 27, 2024

tanderson-ld Aug 28, 2024

kinyoklion Aug 28, 2024

tanderson-ld Aug 28, 2024

kinyoklion Aug 28, 2024

feat: Correct client evaluation typings. #554

feat: Correct client evaluation typings. #554

Conversation

kinyoklion commented Aug 27, 2024 • edited Loading

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

kinyoklion commented Aug 27, 2024 •

edited

Loading