headlamp-k8s / headlamp

A Kubernetes web UI that is fully-featured, user-friendly and extensible
https://headlamp.dev
Apache License 2.0
2.1k stars 150 forks source link

Plugin components face undesired mounting when parent state changes or there is a rerender #323

Open ashu8912 opened 3 years ago

ashu8912 commented 3 years ago

Description

When a plugin component is rendered it faces unwanted mounting whenever there is a rerender caused by its parent component

Impact

The plugin component is in the middle of a state change and suddenly it gets unmounted and remounted with all the state lost

This I believe is occurring because we use plugin components as inline components in our component render cycle and each time there is a rerender the inline component function signature changes and because of reacts shallow comparison on function(which are objects) the component gets unmounted completely

One solution that could prevent this would be we could use useMemo to render the plugin components this way we prevent the unnecessary rerenders as the component will be computed only when the dependency array things change But according to the docs on useMemo it says you should not completely rely on useMemo your component should work without the need for it and useMemo should be used only when you are trying to optimize, and thus a solution without useMemo is something we need right now.

The other thing is according to redux docs we should not store components in our store but we have to store it because redux store is the only way our plugin can communicate with the app world, the question is can there be some alternate approach through which we can prevent this? http://web.archive.org/web/20150419023006/http://facebook.github.io/react/docs/interactivity-and-dynamic-uis.html#what-shouldnt-go-in-state

ashu8912 commented 3 years ago

cc: @joaquimrocha @illume

joaquimrocha commented 2 years ago

@ashu8912 , Is this fixed already, or is there more than #319 that we need to look into? If this is all, then please confirm and close it.

ashu8912 commented 2 years ago

@joaquimrocha we still need to look for a concrete solution to prevent the unnecessary rendering

illume commented 2 years ago

I think we need to change the way we do plugin registration.

As @ashu8912 mentions, we should not store functions inside the redux store.

Components in react have a life cycle, and (un)mount and renders and re-renders are something that should be allowed to happen. It's good that we don't re-render and un(mount) unnecessarily, but components should work despite that. Plugins should really use the react component life cycle.