search

graphql-kit / graphql-voyager

🛰️ Represent any GraphQL API as an interactive graph
https://graphql-kit.com/graphql-voyager/
MIT License
7.76k stars 514 forks source link

fix: move ++ opertator to the front & cleanup useEffect #336

Closed MitchWijt closed 1 year ago

MitchWijt commented 1 year ago

This PR fixes the SVG renderer.

Previous the listener id index was being incremented by doing id++. However this returns the unincremented result. Meaning if id = 0. id++ will return 0. The id never got incremented, causing the function to crash.

Second bug fix is cleaning up the useEffect function in the Voyager component, this was causing errors.

MitchWijt commented 1 year ago

@IvanGoncharov Could you have a look at this whenever you have the time please? It will fix a bug people are having currently rendering graphs

ravar17 commented 1 year ago

@IvanGoncharov , Could you please have a look and approve..

cc @MitchWijt

ravar17 commented 1 year ago

I am so sorry, but we are in the tight deadline to achieve the voyager feature into out backstage developer portal. could you please approve and merge this PR, so that we can TEST. Thanks

@IvanGoncharov , Could you please have a look and approve..

cc @MitchWijt

IvanGoncharov commented 1 year ago

Previous the listener id index was being incremented by doing id++. However this returns the unincremented result. Meaning if id = 0. id++ will return 0. The id never got incremented, causing the function to crash.

@MitchWijt I don't get what the problem is here, can you please explain with more details? id is incrementing and everything is working in my case if you add console.log('id', id); you will see that it gets a new value each time.

IvanGoncharov commented 1 year ago

@MitchWijt This issue is already fixed here: https://github.com/graphql-kit/graphql-voyager/pull/324/files Can you please try this PR?

MitchWijt commented 1 year ago

@IvanGoncharov The problem comes forth over here: https://github.com/backstage/backstage/issues/18736

The isMounted fixes the error Can't perform a React state update on an unmounted component. On every render of the component I was getting this error.

id was not correctly incrementing in my case, causing the listeners array to be invalid, thus rendering of the voyager graph failing. Which you can see in the backstage bug PR

ravar17 commented 1 year ago

@IvanGoncharov @MitchWijt
any updates ?

MitchWijt commented 1 year ago

@IvanGoncharov does the above comment make sense to you?

MitchWijt commented 1 year ago

@IvanGoncharov Will these changes be published to NPM any time soon?

IvanGoncharov commented 1 year ago

@MitchWijt I will try to publish something today. Did you had a chance to test the current main? Is it working for you?

IvanGoncharov commented 1 year ago

@MitchWijt https://github.com/graphql-kit/graphql-voyager/releases/tag/v2.0.0