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

errors during react() while drag-selecting #2644

Closed
nicolaskruchten opened this issue May 17, 2018 · 8 comments
Closed

errors during react() while drag-selecting #2644

nicolaskruchten opened this issue May 17, 2018 · 8 comments

Comments

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented May 17, 2018

I'm trying to upgrade my crossfilter example to be more declarative by using react() but seeing errors. This is my attempt: https://github.com/plotly/plotly.js-crossfilter.js/blob/23b1ff9bd8dc0a3923caf29e4d3feb960ec31234/script.js

While drag-selecting on top of the Infant Mortality chart, the dashed-line selection box does not appear, and errors appear in the console:

plotly-latest.min.js:7 Uncaught (in promise) TypeError: Cannot read property '0' of undefined
    at Object.m.supplyDefaultsUpdateCalc (plotly-latest.min.js:7)
    at Object.r.react (plotly-latest.min.js:7)
    at redraw (script.js:64)
    at n.hist_im_select (script.js:121)
    at n.emit (plotly-latest.min.js:7)
    at HTMLDivElement.t.emit (plotly-latest.min.js:7)
    at plotly-latest.min.js:7
@etpinard
Copy link
Contributor

that one sounds similar to #2602, might have been fixed #2603

@nicolaskruchten
Copy link
Contributor Author

I'm using the latest CDN build so I don't think that's it. It would be really nice for this to at least have the same behaviour as in #2643 ... /cc @alexcjohnson :)

@nicolaskruchten
Copy link
Contributor Author

Ah, for clarity, here is the diff between the two cases: https://github.com/plotly/plotly.js-crossfilter.js/commit/23b1ff9bd8dc0a3923caf29e4d3feb960ec31234

@alexcjohnson
Copy link
Collaborator

Alright, managed to reproduce and understand what's going on here:

  • The user starts a selection on the infant mortality plot
  • The event callback calls Plotly.react on the same plot, giving it new data arrays.
  • This change triggers a "recalc" ie the most complete redraw we do - and the signal we've used for Plotly.plot to know to recalc is to delete gd.calcdata.
  • But Plotly.plot bails out because you're in the middle of a drag:

https://github.com/plotly/plotly.js/blob/master/src/plot_api/plot_api.js#L139-L145

  • Then the next time we call Plotly.react the data arrays may have not changed, so we don't delete gd.calcdata, we expect it to be there to be relinked to the modified traces.
  • Except that we haven't actually DONE a recalc in the meantime (we won't do one until mouseup, though it looks like somehow this one is sometimes getting lost too?), so there's no gd.calcdata
  • 💥

I could do various simple things to avoid the particular error seen here (recalc before bailing out of Plotly.plot, or use some other signal besides deleting gd.calcdata, or just check if gd.calcdata exists before trying to relink it) but all of those have problems of various severity with the plot data being out of sync with the displayed plot, which are going to bite us down the line in obvious or subtle ways.

I think the solution is to make something like gd._hoverData and Plots.rehover but for dragging, so we can get rid of that gd._dragging bail out, actually do the replot, then re-establish the partial drag (be it a pan, a zoom, a select...). Obviously this has the potential to be a bit of a project, given all the different dragmode settings; but it would also address the use case of streaming (or otherwise changing independent of user interaction) data.

@nicolaskruchten we talked about making the select box / lasso part of the figure - and that may still be part of this solution (just as we'd eventually like to have the hover data be part of the figure #1848, but I think the parts of the state that depend explicitly on mouse events (is the mouse up or down, where was it at mousedown) - which are necessary to fully recreate the plot after a mid-drag redraw - should not be part of the figure but should instead be kept as internal state. For one thing we don't really want the user to be able to say "start this plot out as though the mouse is down." For another, having this be internal state that we re-initialize after replot would allow us to clear it naturally if the change has obviated that state, like if the subplot was removed, dragmode was changed, anything like that.

@etpinard also brought up #1640 - an infinite loop created by using the hover event to do something that moves data points around (in that case adding/removing an annotation that changes the axis ranges) such that as soon as a point is hovered it is moved by the update so it's not hovered on, whereupon it's moved back again... I'll comment more over there, but similar loops could be an issue here too.

@etpinard
Copy link
Contributor

I think the solution is to make something like gd._hoverData and Plots.rehover

This sounds like the correct way forward.

we talked about making the select box / lasso part of the figure

Yes. This one is in #1851

(just as we'd eventually like to have the hover data be part of the figure #1848,

Can't wait for that one.

but I think the parts of the state that depend explicitly on mouse events (is the mouse up or down, where was it at mousedown) - which are necessary to fully recreate the plot after a mid-drag redraw - should not be part of the figure but should instead be kept as internal state

I agree 100% here.


On a related topic, now that we're marching towards making every piece of selection interactions part of the plotly.js state, that is selections will soon be "fully" recreatable using restyle and relayout, maybe we should start thinking about dropping plotly_select* family of events and replace them with soon-to-be equivalent restyle/relayout event data?

@etpinard
Copy link
Contributor

etpinard commented May 23, 2018

Stumbled on #2120 which also reports gd._dragged limitations.

@etpinard
Copy link
Contributor

etpinard commented Dec 3, 2018

Merged into #3305

@etpinard etpinard closed this as completed Dec 3, 2018
@amaralc
Copy link

amaralc commented Apr 23, 2021

@nicolaskruchten, I stumbled on a similar issue, but with different origin. I could not find a specific issue for this exact problem, but saw mentions of related issues on #2687 and #3305.

The error is not repetitive, and I still didn't figure out the exact conditions reproduce it. Is there something I could do about it?

Im using plotly.js ^1.58.4, with react-plotly.js ^2.5.1, to render a scatter3d plot that

plotly.js:142641 Uncaught TypeError: Cannot read property '0' of undefined
    at project (plotly.js:142641)
    at Scene.proto.render (plotly.js:142992)
    at Object.scene.glplot.onrender (plotly.js:142943)
    at redraw (plotly.js:50625)
    at render (plotly.js:50740)

The structure of my Plot component is as follows, and the React component is updated when data, layout and config properties change.

    <Plot
      ref={plotElement}
      data={data}
      layout={layout}
      config={config}
      className={classes.waterfall}
      onInitialized={() => {
        setIsChartLoaded(true);
      }}
      onRelayout={handleRelayoutFactory}
      onError={handleChartError}
    />

Detailed browser error view:

Uncaught TypeError: Cannot read property '0' of undefined
    at project (plotly.js:142641)
    at Scene.proto.render (plotly.js:142992)
    at Object.scene.glplot.onrender (plotly.js:142943)
    at redraw (plotly.js:50625)
    at render (plotly.js:50740)

project | @ | plotly.js:142641
-- | -- | --
  | proto.render | @ | plotly.js:142992
  | scene.glplot.onrender | @ | plotly.js:142943
  | redraw | @ | plotly.js:50625
  | render | @ | plotly.js:50740
  | requestAnimationFrame (async) |   |  
  | render | @ | plotly.js:50741
  | createScene | @ | plotly.js:50745
  | proto.tryCreatePlot | @ | plotly.js:142796
  | proto.initializeGLPlot | @ | plotly.js:142850
  | Scene | @ | plotly.js:142736
  | plot | @ | plotly.js:141668
  | exports.drawData | @ | plotly.js:123232
  | lib.syncOrAsync | @ | plotly.js:112023
  | plot | @ | plotly.js:117778
  | newPlot | @ | plotly.js:118048
  | react | @ | plotly.js:120084
  | (anonymous) | @ | factory.js:82
  | Promise.then (async) |   |  
  | updatePlotly | @ | factory.js:73
  | componentDidMount | @ | factory.js:110
  | commitLifeCycles | @ | react-dom.development.js:19814
  | commitLayoutEffects | @ | react-dom.development.js:22803
  | callCallback | @ | react-dom.development.js:188
  | invokeGuardedCallbackDev | @ | react-dom.development.js:237
  | invokeGuardedCallback | @ | react-dom.development.js:292
  | commitRootImpl | @ | react-dom.development.js:22541
  | unstable_runWithPriority | @ | scheduler.development.js:653
  | runWithPriority$1 | @ | react-dom.development.js:11039
  | commitRoot | @ | react-dom.development.js:22381
  | finishSyncRender | @ | react-dom.development.js:21807
  | performSyncWorkOnRoot | @ | react-dom.development.js:21793
  | (anonymous) | @ | react-dom.development.js:11089
  | unstable_runWithPriority | @ | scheduler.development.js:653
  | runWithPriority$1 | @ | react-dom.development.js:11039
  | flushSyncCallbackQueueImpl | @ | react-dom.development.js:11084
  | flushSyncCallbackQueue | @ | react-dom.development.js:11072
  | flushPassiveEffectsImpl | @ | react-dom.development.js:22883
  | unstable_runWithPriority | @ | scheduler.development.js:653
  | runWithPriority$1 | @ | react-dom.development.js:11039
  | flushPassiveEffects | @ | react-dom.development.js:22820
  | (anonymous) | @ | react-dom.development.js:22699
  | workLoop | @ | scheduler.development.js:597
  | flushWork | @ | scheduler.development.js:552
  | performWorkUntilDeadline | @ | scheduler.development.js:164
  
  ```

![image](https://user-images.githubusercontent.com/32170099/115905039-ed86eb00-a43b-11eb-933c-fd92ced5daee.png)


![image](https://user-images.githubusercontent.com/32170099/115905153-14452180-a43c-11eb-8125-86f738cd1d74.png)

![image](https://user-images.githubusercontent.com/32170099/115905194-232bd400-a43c-11eb-932f-4e72bd6c4371.png)


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

4 participants