Skip to content
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

[app] Imporove federated module core #405

Merged
merged 2 commits into from
Jul 31, 2022

Conversation

marwonline
Copy link
Contributor

@marwonline marwonline commented Jul 31, 2022

Small refactoring of the module federation handling which is used to load/handle/initialize plugins

⚠️ This needs to be tested on a real Kubernetes cluster.

@marwonline marwonline force-pushed the app-fed-mod-refactoring branch 2 times, most recently from 89606a3 to 67ba10a Compare July 31, 2022 09:07
* @param scope the module scope. In kobs it's usually the plugin name: e.g. 'kiali'
* @param module the name of the module entry point. e.g. './Page'
* @returns the module component or method
*/
Copy link
Contributor Author

@marwonline marwonline Jul 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some documentation. In a later PR, I'll try to type this with generics so that we can get rid of any.

}

if (failed) {
if (isFailed) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've re-ordered the cases so that the error case always apprears first.

* Note: the version can't be changed during runtime.
* @returns the status of the script/module
*/
export const useDynamicScript = (name: string, version: string): ScriptStatus => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've separated the Kobs business logic and the script loading into two functions. Should make it easier to re-use it later

@marwonline marwonline marked this pull request as ready for review July 31, 2022 09:12
Copy link
Member

@ricoberger ricoberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @marwonline and thanks for your contribution.

I tested your changes locally and everything works as expected 🙂

@ricoberger ricoberger merged commit e701006 into kobsio:main Jul 31, 2022
@marwonline marwonline deleted the app-fed-mod-refactoring branch August 16, 2022 12:06
@ricoberger ricoberger added the changelog: changed Something was changed or updated label Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: changed Something was changed or updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants