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

Potential issue with layoutEngineType #296

Open
dsebastian opened this issue Jan 26, 2021 · 3 comments
Open

Potential issue with layoutEngineType #296

dsebastian opened this issue Jan 26, 2021 · 3 comments

Comments

@dsebastian
Copy link

Hi, I was trying out react digraph with some streaming data. Upon investigating some performance issues, I stumbled upon the layoutEngineType property which is heavily used throughout the Graph View in order to check if nodes should be re-rendered. One of the methods that tells the component if it should forceUpdate is hasLayoutEngine
https://github.com/uber/react-digraph/blob/master/src/components/graph-view.js#L294
https://github.com/uber/react-digraph/blob/master/src/components/graph-view.js#L331

Considering the type of this property is LayoutEngineType and that it is defaulted to LayoutEngineType.NONE, which in it's self is of value 'None', can anyone tell me when will this method return false as I'm probably missing some thing here? Isn't forceUpdate set to true always?

Thanks.

image

@ajbogh
Copy link
Contributor

ajbogh commented Jan 26, 2021

This checks for the existence of the attribute. If you don't want to run it then don't add the layoutEngineType attribute to the <Graph>. Don't use "None" as the layoutEngineType, just remove the attribute altogether.

The forceRerender is kind of a hack, I know, but it fixed a rendering issue.

@dsebastian
Copy link
Author

Thanks for the reply @ajbogh . But as I was mentioning, the component sets is as a defaultProp here:
https://github.com/uber/react-digraph/blob/master/src/components/graph-view.js#L80

@Djazouli
Copy link
Contributor

Hello,
Indeed, there is "None" defaultProp, so just removing the attribute altogether will indeed set a layout engine. But passing layoutEngineType={null} wont.

I hope it helps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants