-
-
Notifications
You must be signed in to change notification settings - Fork 658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract playground steps #3966
Extract playground steps #3966
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
@@ -21,7 +21,7 @@ export const generateCombinations = <T extends Record<string, string>>( | |||
) as T[]; | |||
}; | |||
|
|||
export const generateObjectCombinations = <T extends Record<string, string>>( | |||
export const generateObjectCombinations = <T extends Record<string, any>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will persist the input type instead of forcing string
@@ -64,7 +64,7 @@ export const offlineUnleashClient = async ({ | |||
}, | |||
}); | |||
|
|||
client.start(); | |||
await client.start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start is async so await is needed. w/o it the code would break if we switched to node.js sdk
@@ -68,39 +92,66 @@ export class PlaygroundService { | |||
? new Date(context.currentTime) | |||
: undefined, | |||
}; | |||
const output: PlaygroundFeatureSchema[] = await Promise.all( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extracting into a shared method that will be reused in advanced query evaluation
|
||
private async evaluate({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is extract method refactoring - no new code in here
|
||
return output; | ||
} | ||
} | ||
|
||
private async resolveFeatures( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract method refactoring - no new code in here
); | ||
const contexts = generateObjectCombinations(context); | ||
|
||
const results = await Promise.all( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be thoroughly tested in subsequent PRs. Here I wrote this to manually test the combinatorial generation and drive the extract method refactoring
import { FeatureConfigurationClient } from '../../types/stores/feature-strategies-store'; | ||
import { generateObjectCombinations } from './generateObjectCombinations'; | ||
|
||
type EvaluationInput = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
introducing a type to capture what is actually used for the playground evaluation. Those are the values that we derive from the user input by going to our stores
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
About the changes
Important files
Discussion points