infinite loops still possible despite prop checks

[mattearnshaw]

the README example code for onUpdate hangs when transplanted into my project. the reason being simply that in general {a:1} === {a:1} evaluates to false. It looks like a good option would be to extirpate these fudged "shouldUpdate?" conditions from componentWillUpdate altogether and change PlotlyComponent to a PureComponent. Verbatim this doesn't quite do the trick: I think .then(() => this.figureCallback(nextProps.onUpdate)) needs to go somewhere else too, but I haven't figured it out.

related #77

[nicolaskruchten]

Yeah, there's some brittleness here for sure. Can you provide a bit more context about what's failing in your app?

[mattearnshaw]

@nicolaskruchten I'm using the example code more or less as written, but with a big custom layout [redacted]

this causes nextProps.layout === this.props.layout && nextProps.data === this.props.data && nextProps.config === this.props.config && numNextFrames === numPrevFrames to always evaluate to false, thus hanging the browser.

[nicolaskruchten]

Yeah, the subtlety there is that in the example code, the value of the prop is always the same: it's a lookup in state, whereas in your code, you're passing in a new layout each time. If you restructure your code to have layout's identity be persistent except for immutable changes, then things work as expected.

This is documented here but clearly I wrote that section more in a "here's how to trigger an update" and not "here's how to avoid triggering an update" manner, and I should make it clearer.

[mattearnshaw]

ah yes, I should have noticed this, thanks. it's working as expected now (once I jammed the data into state too). I'm not sure if you want to still treat this as an open issue, but close if you see fit.

[nicolaskruchten]

Let me find a sec to update the readme to call this out specifically and maybe you can give me some feedback on the phrasing to see if it would have helped you avoid this issue?

[nicolaskruchten]

OK so this isn't going to be a one-liner in the documentation after all... Closing in favour of #88.

Thanks for the nudge to clear up this documentation weirdness, and sorry you had to encounter this problem before I realized we were leading everyone astray like this!