Skip to content

stop using componentWillUpdate #77

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

Closed
alexcjohnson opened this issue May 4, 2018 · 3 comments
Closed

stop using componentWillUpdate #77

alexcjohnson opened this issue May 4, 2018 · 3 comments

Comments

@alexcjohnson
Copy link
Collaborator

Currently we update the plot in componentWillUpdate - it works, but it's a little weird and this method is deprecated.

  • Put the conditionals preventing updates into shouldComponentUpdate?
  • Move the update itself to componentDidUpdate
  • The chain of events in componentDidMount (initial draw) and the update code are a little different, but there may be some opportunity to DRY this up - at least, the Plotly.newPlot in the initial draw can safely be turned into Plotly.react.
@dmt0
Copy link
Contributor

dmt0 commented Feb 20, 2019

@alexcjohnson if we switch to using Plotly.react, would we need to bump plotly.js peer dependency which is currently set as: "plotly.js": ">1.34.0"

@alexcjohnson
Copy link
Collaborator Author

No, v1.34.0 is in fact the version that introduced Plotly.react... which makes sense since the fallback code for missing Plotly.react has already been removed.

@nicolaskruchten
Copy link
Contributor

Yes merging the logic into DidUpdate and getting rid of newPlot seems like a winner to me

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