search

n3m3sis00 / phylo-react

React component library for Phylogenetic Analysis
https://n3m3sis00.github.io/phylo-react/
1 stars 1 forks source link

Endless Re-rendering Loop due to callback of Tree component #41

Open n3m3sis00 opened 3 years ago

n3m3sis00 commented 3 years ago

currently get config is designed to receive a set state method but due to this parent component of Tree keeps attaching endlessly without using some hacky fix like

<Tree 
      data = {treedata}
      layout = {layout}
      getConfig={treeresponse === null ? setTreeresponse : d => {}}
/>
cmdcolin commented 3 years ago

is there an example page that has an infinite loop?

n3m3sis00 commented 3 years ago

I created a demo app to investigate the above bug please have a look LINK : https://n3m3sis00.github.io/phylo-demo/

it behave somewhat like this consolelog

cmdcolin commented 3 years ago

What is the source code for this example? I see you have https://github.com/n3m3sis00/phylo-demo but this is the compiled source code

The reason this might be an infinite loop is that every time getConfig is called for some purpose, then Tree re-renders. There are ways to avoid re-rendering including

a) finding out why Tree re-rendered, can use simple tools like this https://usehooks.com/useWhyDidYouUpdate/ b) architecting the app different so we do not perform getConfig (this is a bit of an anti pattern in the one-way-data flow that most react apps have, there should not be a side-effect to rendering like this)

cmdcolin commented 3 years ago

I think it might boil down to re-architecting the app though, now that I phrase it this way. We could extract some of the d3 logic for building the tree with cluster, etc. to a new utility function, and then using that in tree and MSA (that way getConfig is not needed at all)

cmdcolin commented 3 years ago

Also can you make this TreeMSA example into a storybook story?

n3m3sis00 commented 3 years ago

What is the source code for this example? I see you have https://github.com/n3m3sis00/phylo-demo but this is the compiled source code

https://github.com/n3m3sis00/phylo-demo/tree/master | i don't know why gh-pages become the default branch :(

The reason this might be an infinite loop is that every time getConfig is called for some purpose, then Tree re-renders.

yes yes,

We could extract some of the d3 logic for building the tree with cluster, etc. to a new utility function, and then using that in tree and MSA (that way getConfig is not needed at all)

This is a great idea, this will also reduce the overburden of attaching Tree component when it is not required, like in solo synteny or MSA View.

Also can you make this TreeMSA example into a storybook story?

Sure I will do that :)