-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: switch and parallel running for DUP logic #2201
Conversation
5a8700f
to
98a258a
Compare
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.
Very interesting solution. I'm a fan. Assuming the DUP packaging process still outputs a working app, this is great. Just have a couple of non-blocking comments.
// such as `return { ...response, data: response.variants[variant] }`, but | ||
// the compiler can't work out that the types are compatible. Maybe check | ||
// this again once we upgrade to TypeScript 5. | ||
const copy = { ...response }; |
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.
We already have lodash available so maybe could replace this with _.set
or a similar function?
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.
I'm not quite getting how that would simplify things, considering we shouldn't mutate response
itself here since it is a React-managed state value.
98a258a
to
75c23da
Compare
Good call on testing the DUP packaging process, I hadn't done that before so it was also a good opportunity to see how it works. It looks like the package still works as it should! |
Introduces the concept of a "variant", which is an alternate candidate generator for an app ID. Also introduces a variant for DUPs which will be used to build and test the new departures logic. For now, this only generates a set of placeholder elements. Variants can be enabled via: * Adding a `variant=NAME` query param to a screen URL. The resulting screen page will fetch and display data for only that variant. * Enabling the "variant switcher" in the admin Inspector. This allows toggling between variants instantly without refreshing data; the data hook uses a special `all` value to request and hold onto the data for all variants at once. Additionally, `ScreenData` functions have an option to run and serialize all variants that exist for the app ID in parallel, in the background, without waiting for or doing anything with the results. This means we can have all existing screens constantly running the new logic at every step of the implementation, giving us more confidence that the final rollout would go smoothly. For now, we only use this option on "normal" screen requests (not pending, not simulation).
75c23da
to
2f6eb78
Compare
Introduces the concept of a "variant", which is an alternate candidate generator for an app ID. Also introduces a variant for DUPs which will be used to build and test the new departures logic. For now, this only generates a set of placeholder elements.
Variants can be enabled via:
Adding a
variant=NAME
query param to a screen URL. The resulting screen page will fetch and display data for only that variant.Enabling the "variant switcher" in the admin Inspector. This allows toggling between variants instantly without refreshing data; the data hook uses a special
all
value to request and hold onto the data for all variants at once.Additionally,
ScreenData
functions have an option to run and serialize all variants that exist for the app ID in parallel, in the background, without waiting for or doing anything with the results. This means we can have all existing screens constantly running the new logic at every step of the implementation, giving us more confidence that the final rollout would go smoothly. For now, we only use this option on "normal" screen requests (not pending, not simulation).