Comments (17)
You asked for how this caused a problem in our case, and while the state example I gave was a simple example of where things could go wrong when props are mutated, it perhaps did not describe a realistic scenario very well :)
The problem we have run into was a bit more complex. But basically we have a page with a list of objects, lets call them processes. When clicking on a process, the user is routed to another page displaying a plotly graph of the process status.
The data comes from a web API through the redux store, while the layout is a template object constant, that only sometimes is modified depending on the data.
Both are fed to a Wrapper component containing the plotly Plot component.
Going back from the plot page to the list page, the Wrapper and Plot components are unmounted and destroyed.
Now, going to another plot, using the same layout object (which originates higher up in the component tree, and thus not destroyed) we may suddenly have extra properties like selected tool or zoom range (depending on how the user interacted with the last plot) in our newly created plot that we did not expect.
Now that we know that props are mutated, it should be possible to work around until you have a longterm solution to what I understand is a complex problem. However, in the meantime, would it be possible to add this information to the documentation, e.g. in the Plotly component API?
from react-plotly.js.
To second @gimbo's comment, we got only blank space where the plot was supposed to be after refactoring to use redux-toolkit (which uses immer under the hood). This was part of a larger refactoring, so it took some time to figure out the problem — no errors logged, but the debugger revealed it of course.
In this case, the workaround is to disable immer's auto freeze functionality:
import { setAutoFreeze } from 'immer';
setAutoFreeze(false);
from react-plotly.js.
Just a small heads-up...
I've just hit this problem using a Plot
component in a React/Redux app built using the Redux Starter Kit which, it seems, the redux docs will soon be encouraging newcomers to use; that kit includes the redux-immutable-state-invariant package by default (in dev mode anyway), which detects these layout/data mutations and turns them into showstopping errors. (Fortunately I figured out that deep-cloning the layout
prop "fixed" the problem, which led me to this issue — glad I didn't spend a couple of hours debugging redux-observable epics, which was my first thought as to the culprit!)
Anyway, yeah, you might see more people hitting this in future if Redux Starter Kit gets traction, which I guess it will if the Redux docs point to it, which they almost certainly will...
from react-plotly.js.
Probably just "here is the final state", because the instructions would basically be "feed this back to me please" :)
One thing to note here is that for versions of plotly.js before 1.34, this type of wiring would cause many updates to result in 2 newPlot
calls :(
from react-plotly.js.
Thank you for sharing, this makes the problem quite clear. I apologize that our unclear documentation caused you undue issues in tracking this down and I'll see about making it clearer. In the short run, however, our goal is to actually fix this, obviously :)
from react-plotly.js.
Yes, this is a tricky situation, because plotly.js does mutate its inputs, whereas this is unexpected/discouraged behaviour in React. I'm not sure what the best way forward is here: if the React wrapper made a deep copy of layout
before every update then Plotly.react
's diffing logic wouldn't work, right @alexcjohnson ?
from react-plotly.js.
Tricky indeed. We've had some discussions about how to handle this but haven't settled on anything yet. It would be OK for the wrapper to deep copy layout
and data
(yes, data
can get mutated too in some cases) before passing them on to Plotly.react
except that you'd need to skip data_array
items. The challenge then is that the copy of the plot state that you're holding doesn't match what's actually on the plot. Note also that there are some mutations that happen during the react
/newPlot
call, and these we don't report back in any way (ie by emitting an event), while mutations generated later via UI interactions are reported via restyle
/relayout
events.
I feel like the way forward is going to be something like:
- Add a
config
parameter to plotly.js telling us to managedata
andlayout
immutably - some users depend on the existing behavior so we can't change the default until v2. But when that parameter is used: - All changes to
data
/layout
are done immutably, meaninggd.data
andgd.layout
become new objects as necessary. react
/newPlot
end by checking if these objects have changed (which at that point is just===
), and if so reporting them back (as a new event)?restyle
/relayout
/update
also switch to immutable updates. I guess since these methods already emit events, we wouldn't necessarily need to emit the "mutation" event, though perhaps it would be easier if we did - so the react wrapper would only need to listen to the one event?- On receiving this event, the react wrapper updates its own state. If this means that state gets passed back into
Plotly.react
that's fine - it means we'll diff again, find no changes, and do nothing more, so the only penalty is the diff itself. Actually, perhaps since the immutableconfig
parameter is set, even the diff can be avoided by just seeing thatdata === gd.data
andlayout === gd.layout
.
from react-plotly.js.
The config
parameter for immutable updates is just what I suggested to @etpinard this morning :)
That said, I think that the implementation suggest above will not resolve the main issue here unfortunately... Take a case where layout = {a: {b: 1}}
and you want to set layout.a.b = 2
. If you just change the identity of layout
to a new object newLayout
whose a
key is a reference to layout.a
then layout.a == newLayout.a
and/but newLayout.a.b
's value will have mutated, meaning that any code elsewhere which has grabbed a reference to a piece of layout
can't use simple equality checks against the corresponding piece of newLayout
. To avoid this you need the whole path from layout
to any mutated key to become a new object for every mutation. This is basically what something like https://github.com/kolodny/immutability-helper does.
I must say that I also don't really understand the utility of subscribing to events and then calling Plotly.react
again? The wrapper gets its state from upstream, but it's never read by anyone downstream, and it cannot push these changes back upstream (this goes against the whole React one-way-dataflow principle), so what is the downside to just calling Plotly.react
and leaving it at that?
from react-plotly.js.
On the other hand, while I know that this React component breaks React's single strict rule, I do want to inquire a bit about the actual concrete consequences today for users of the component... @swiperii where does this cause problems for your app? Are you relying on state
comparisons elsewhere? My expectation would be that your app is still free to overwrite state whenever it likes and the component would update to match the new state etc. I'm not sure what the failure case is and I would love more information about it if you could provide some :)
from react-plotly.js.
To avoid this you need the whole path from
layout
to any mutated key to become a new object for every mutation. This is basically what something like https://github.com/kolodny/immutability-helper does.
Yes, that's what I had in mind. This kind of situation doesn't happen all that much, so I'm not worried about its overhead, but we would need to construct a new layout
object all the way up the tree from the mutated value.
I must say that I also don't really understand the utility of subscribing to events and then calling
Plotly.react
again? The wrapper gets its state from upstream, but it's never read by anyone downstream, and it cannot push these changes back upstream (this goes against the whole React one-way-dataflow principle), so what is the downside to just callingPlotly.react
and leaving it at that?
Right, I don't know what the right way of doing this is, but one way or another we need to be able to feed state back into the app from the plot. Certainly just closing this loop in the wrapper wouldn't solve the problem, but perhaps the wrapper can usefully mediate between the plot and the app? Other parts of the app could need to know when the user zooms in on the plot, for example.
I wasn't really thinking of subscribing to the event just to call Plotly.react
again - I was just imagining that the plot state is changed, one way or another this needs to be pushed back into the app state, then the app passes it back down to the plot wrapper, so automatically Plotly.react
gets called again. This part I'm also not worried about, because Plotly.react
will quickly (and perhaps almost instantaneously since we have the immutable config arg) notice that it's seen this whole state before.
from react-plotly.js.
OK, so I'm not sure why "one way or another we need to be able to feed state back into the app from the plot" ?
from react-plotly.js.
Extra context: we already accept handlers for these events https://github.com/plotly/react-plotly.js/blob/master/src/factory.js#L10 ... but if Plotly.react
mutates its inputs why would the upstream code need to know?
from react-plotly.js.
PS: 👍 to the "all the way up the tree" approach to immutability... it wasn't clear from your initial comment if that's what you mean, so I'm glad we're on the same page!
from react-plotly.js.
Extra context: we already accept handlers for these events
Yeah, so maybe all I'm suggesting is a catch-all "state changed" handler so you don't have to worry about all these individual events, but also you get any changes made just by the process of drawing the plot.
if
Plotly.react
mutates its inputs why would the upstream code need to know?
I think these are all "you asked for auto-something, here's the something you got." I guess the main reason to want to pass these upstream is that update performance will be better if we don't have to recalculate those auto values (particularly autorange, and zmin/zmax for heatmaps and related types), but if they're not in the new state you pass in, we will have to recalculate them which triggers a slower update pathway. There may be a way for us to change plotly.js to avoid this problem, but that's a big project and possibly not even something we can do until v2, I'm not sure. Secondarily, other parts of your app may want to make use of these values. I guess for cases like this your app can look at _fullLayout
and _fullData
, but it could be even easier if they're already part of the app state.
from react-plotly.js.
Ahhh I see. I'm always sensitive to performance-based reasons :)
Re the catch-all, the current wrapper does fire props.onUpdate()
for the following plotly.js events: https://github.com/plotly/react-plotly.js/blob/master/src/factory.js#L37 ... Does Plotly.react
fire those appropriately at the moment or are some changes required? Or do we need a new event type?
from react-plotly.js.
No, mutations that happen automatically during drawing do not generate any events right now. What would it take to make an event like this useful? Is it enough to just flip a flag somewhere and say "something changed during drawing, here's the final state," or is it important to know what changed?
from react-plotly.js.
@jobh Thank you so much for sharing your solution. If it wasn't you I will still keep searching why I get empty graph for ages.
from react-plotly.js.
Related Issues (20)
- How do I configure spikedistance? HOT 1
- Depreciated in favor of Angular? HOT 2
- Update plotly.js to include the latest version HOT 1
- onDoubleClick does not work in 3D mesh HOT 1
- Support Synchronizing of zoom & hover for multiple charts on the same page HOT 1
- ranges does not get computed when same data is used but Plot is updated
- Can't resolve 'react-plotly.js' in HOT 1
- onUpdate - reset the zoom in Nextjs (React) HOT 2
- Rangeselector buttons flashing
- Plotly.restyle breaks onClick handlers
- Can't disable sunburst default animation HOT 1
- Is layout.height really disallowed when layout.autosize=true ?
- Axis `automarging` does not work HOT 6
- Need to extract `.default` before using `createPlotlyComponent` HOT 1
- Request for Kernel Density Estimate Plot Feature in react-plotly HOT 1
- Chart is cropped. HOT 1
- How can I use animate method ?
- How do I use Plotly.Fx.hover with react-plotly.js?
- Heatmap not showing HOT 1
- Clickmode "event+select" activated and Clicking outside of Graph calls onUpdate incorrectly
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-plotly.js.