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

Restore compatibility with React 15 #237

Closed
bhgsbatista opened this issue Sep 17, 2019 · 6 comments
Closed

Restore compatibility with React 15 #237

bhgsbatista opened this issue Sep 17, 2019 · 6 comments

Comments

@bhgsbatista
Copy link
Contributor

bhgsbatista commented Sep 17, 2019

Are you reporting a bug, or opening a feature request?

Bug

What is the actual behavior/output?

Translate prop does not seem to do anything when using React 15.

What is the behavior/output you expect?

Translate prop would translate SVG with React 15.

Can you consistently reproduce the issue/create a reproduction case (e.g. on https://codesandbox.io)?

Yes: https://codesandbox.io/s/nifty-wilson-xbfl8

What version of react-d3-tree are you using?

1.15.0

@bhgsbatista
Copy link
Contributor Author

This seems to have been broken after version 1.13.0

@bhgsbatista
Copy link
Contributor Author

Actually I think this is related to adding UNSAFE_ in front of React lifecycle hooks, since I'm still on React 15 this broke things for me.

@bhgsbatista
Copy link
Contributor Author

bhgsbatista commented Sep 17, 2019

This PR restores compatibility with React 15 and 16: #239

I think dropping support for React 15.x should be done in a major version, ex: 2.0.0

@bhgsbatista bhgsbatista changed the title Translate prop not working Restore compatibility with React 15 Sep 18, 2019
@bkrem
Copy link
Owner

bkrem commented Sep 18, 2019

Yeah this was completely unintentional 🤦‍♂ thanks for reporting it and opening a PR to revert the changes.

Since I wanted to solve the compat issue (at least for now) for both people on React 16 and React 15, I've migrated or simplified the affected code and polyfilled with https://github.com/reactjs/react-lifecycles-compat where the compat constraints couldn't be satisfied with componentDidUpdate alone.

I published those changes as v1.15.1.

Thanks for your help again, please reopen this if compat issues with React 15 persist 👍

@bhgsbatista
Copy link
Contributor Author

Thanks for fixing the issue promptly and the correct way :)

@bhgsbatista
Copy link
Contributor Author

bhgsbatista commented Sep 18, 2019

The issue is still occurring: https://codesandbox.io/s/nifty-wilson-xbfl8

Once you pan even once the tree jumps to the correct destination, I think the issue is this line in componentDidUpdate:

this.internalState.d3 = Tree.calculateD3Geometry(this.props);

Because setState is not called here we do not re-render with the correct d3 state.

I think it would be a better to store d3 inside state, not a separate internalState member. That way it could be generated inside getDerivedStateFromProps.

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

2 participants