feat: Convert optimizely_config module to TS #626
Conversation
| public revision: string; | ||
| private datafile: string; | ||
|
|
||
| constructor(configObj: OptimizelyConfigOptions, datafile: string) { |
There was a problem hiding this comment.
I think the type of configObj should be ProjectConfig from the project_config module (link). Using this might also remove the need to create some of the interfaces above.
| featureKeyMap: { [key: string]: FeatureFlag }; | ||
| } | ||
|
|
||
| interface ExperimentIds { |
There was a problem hiding this comment.
I don't love the name ExperimentIds for this. It is a mapping from experiment ID to boolean. Since it's only used in one place I'm not sure it needs to be declared as an interface.
| * @param {Rollout[]} rollouts | ||
| * @returns {ExperimentIds} Experiment Ids which are part of rollout | ||
| */ | ||
| private getRolloutExperimentIds(rollouts: Rollout[]): ExperimentIds { |
There was a problem hiding this comment.
For getRolloutExperimentIds, getExperimentsMap, getMergedVariablesMap, and getFeaturesMap, I don't think they need to be class methods. They don't access any data from this, they just use arguments. I suggest leaving them as separate (unexported) functions in this module, or making them static methods.
Summary
optimizely_configmodule to TSTest plan
Existing unit and fsc tests and running local bundle
Issues