feat: Create index.d.ts file for decision_service #568

yavorona · 2020-09-15T19:07:54Z

Summary

Create index.d.ts file for decision_service module

mikeproeng37 · 2020-09-15T19:11:10Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

@@ -0,0 +1,151 @@
+/**
+ * Copyright 2018-2020, Optimizely


Should just be 2020

mikeproeng37 · 2020-09-15T19:11:29Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+  }
+
+  export interface ConfigObj {
+    __datafileStr: string;


Shouldn't document private properties

mikeproeng37 · 2020-09-15T19:12:06Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+    featureKeyMap: {[key: string]: FeatureFlag}; //look into this one closer
+  }
+
+  interface Event {


A lot of these entities are being defined in #567 already. We should reuse them.

Agree, for all the entity types that are related to or located in project config.

coveralls · 2020-09-15T19:13:59Z

Coverage remained the same at 96.638% when pulling 3dcda32 on pnguen/decision_service_index.d.ts into eaf61d0 on master.

mjc1283

Requesting changes to locate type definitions in the appropriate module and import them into the consumers.

mjc1283 · 2020-09-16T22:35:45Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+    UNSTABLE_conditionEvaluators: unknown;
+  }
+
+  interface ConfigObj {


This should be imported from the project config module (see PR #567)

mjc1283 · 2020-09-16T22:36:39Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+
+  export function createDecisionService(options: Options): DecisionService;
+
+  export interface UserProfileService {


Are UserProfileService and/or UserProfile needed in any other modules? There might be a more natural place for these to live.

I moved them back per your comment on UserProfileService. Although, now we are importing them from a root-level module to here, which is an anti-pattern. What are your thoughts @mjc1283?

We are importing them from a root-level module to here, which is an anti-pattern.

Agreed. Let's keep them in decision service like you originally had, and consider moving them later on if it makes sense.

Sounds good. UserAttributes would also need to be moved here for now to avoid anti-pattern.

mjc1283 · 2020-09-16T22:37:14Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+    featureKeyMap: {[key: string]: FeatureFlag}; //look into this one closer
+  }
+
+  interface Event {


Agree, for all the entity types that are related to or located in project config.

mjc1283 · 2020-09-16T22:38:24Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+    value: string | number | boolean | null;
+  }
+
+  type ConditionTree<Leaf> = Leaf | unknown[];


Should be imported from condition_tree_evaluator

packages/optimizely-sdk/lib/index.d.ts

mjc1283

LGTM but this depends on #567. If you want to merge it first, let's put more of a stub or skeleton in project_config/index.d.ts which can be filled in later by #567

mjc1283 · 2020-09-17T17:10:34Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+
+  export function createDecisionService(options: Options): DecisionService;
+
+  export interface UserProfileService {


We are importing them from a root-level module to here, which is an anti-pattern.

Agreed. Let's keep them in decision service like you originally had, and consider moving them later on if it makes sense.

mjc1283 · 2020-09-17T17:13:12Z

packages/optimizely-sdk/lib/core/project_config/index.d.ts

+    }>
+  } 
+
+type ConditionTree<Leaf> = Leaf | unknown[];


Import from condition_tree_evaluator

mjc1283

Still waiting for update after #567 is complete

mjc1283 · 2020-09-23T22:45:30Z

packages/optimizely-sdk/lib/core/decision_service/index.d.ts

+  interface Decision {
+    experiment: import('../../shared_types').Experiment | null;
+    variation: import('../../shared_types').Variation | null;
+    decisionSource: string;


We might want to use an enum for this. Can be updated later if so.

yavorona requested a review from a team as a code owner September 15, 2020 19:07

yavorona self-assigned this Sep 15, 2020

mikeproeng37 reviewed Sep 15, 2020

View reviewed changes

yavorona removed their assignment Sep 16, 2020

optimizely-codereview-bot requested a review from mikeproeng37 September 16, 2020 16:12

mjc1283 suggested changes Sep 16, 2020

View reviewed changes

optimizely-codereview-bot assigned yavorona Sep 16, 2020

mjc1283 suggested changes Sep 17, 2020

View reviewed changes

yavorona removed their assignment Sep 17, 2020

optimizely-codereview-bot requested a review from mjc1283 September 17, 2020 17:50

optimizely-codereview-bot added 🤖 4 hrs 🤖 8 hrs 🤖 12 hrs 🤖 16 hrs and removed 🤖 4 hrs 🤖 8 hrs 🤖 12 hrs labels Sep 17, 2020

mjc1283 suggested changes Sep 21, 2020

View reviewed changes

optimizely-codereview-bot removed the 🤖 16 hrs label Sep 21, 2020

optimizely-codereview-bot assigned yavorona Sep 21, 2020

yavorona and others added 6 commits September 23, 2020 13:30

Create index.d.ts file

7ea17f3

Define DecisionService and createDecisionService interfaces

7b0a28d

Clean up

a9289ff

index.d.ts added for project_config

c83b2d2

Fix imports

a0805d7

Fix imports to avoid anti-pattern

2b495c5

yavorona force-pushed the pnguen/decision_service_index.d.ts branch from 21d7df7 to 2b495c5 Compare September 23, 2020 20:40

yavorona added 2 commits September 23, 2020 14:22

Add comments and clean up exports

bf13895

Add one more comment

89856e7

Add missing attribute

3dcda32

yavorona removed their assignment Sep 23, 2020

optimizely-codereview-bot requested a review from mjc1283 September 23, 2020 21:34

mjc1283 approved these changes Sep 23, 2020

View reviewed changes

yavorona merged commit 7fdb06a into master Sep 23, 2020

yavorona deleted the pnguen/decision_service_index.d.ts branch September 23, 2020 22:51


		export function createDecisionService(options: Options): DecisionService;

		export interface UserProfileService {

feat: Create index.d.ts file for decision_service #568

feat: Create index.d.ts file for decision_service #568

Uh oh!

Conversation

yavorona commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

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!

coveralls commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjc1283 left a comment

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!

yavorona Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

mjc1283 left a comment

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!

mjc1283 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

yavorona commented Sep 15, 2020 •

edited

Loading

coveralls commented Sep 15, 2020 •

edited

Loading

yavorona Sep 17, 2020 •

edited

Loading