Fix dynamic loading of Experiment Engine config #205
Conversation
| const [configFailed, setConfigFailed] = useState(false); | ||
|
|
||
| // Retrieve script from host dynamically | ||
| const { ready, failed } = useDynamicScript({ |
There was a problem hiding this comment.
Shall we replace this with LoadDynamicScript too instead of directly calling the hook?
Also, since we have similar logic in multiple places now, there is likely some scope for refactoring this further to reduce some code duplication by wrapping all the loading into a single component.
krithika369
left a comment
krithika369
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the fix and the refactoring!
| }; | ||
|
|
||
| // Dynamic Script Loading component wrapper | ||
| export const DynamicHookComponent = ({ |
There was a problem hiding this comment.
The changes are great! My only comment for improvement is, DynamicHookComponent is named and placed (under ui/src/components/remote_component) as though it were a generic component. It's weird that it worries about the details of url vs config and talks about "Experiment Engine". Can we move this component to ui/src/components/experiments and rename it to ExperimentEngineLoaderComponent or something like that?
2 participants
What this PR does / why we need it:
A previous related PR caused a bug which crashes Routers Details view. This view uses
ExperimentConfigSectioncomponent which requires configured Experiment Engine information retrieved via global variables configured on the Experiment Engine UI. This resulted in runtime configs being NOT loaded into the index.html headers and hence, not available for use.Logs show the following information:
This PR addresses the above issue for both
ExperimentConfigSectionandStandardExperimentConfigGroupcomponents which load information from the remote Experiment Engine UI.