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

Plotly.js requires gl in Electron contexts #891

Closed
rgbkrk opened this issue Aug 31, 2016 · 25 comments
Closed

Plotly.js requires gl in Electron contexts #891

rgbkrk opened this issue Aug 31, 2016 · 25 comments

Comments

@rgbkrk
Copy link

rgbkrk commented Aug 31, 2016

When using plotly.js in an Electron app, it requires gl. I'm guessing this is because Plotly.js determines that it's running in a node context, even though there is a window and a full browsing environment available. Installed gl for now, figured I'd post an issue. I'll link to the relevant PR on the nteract side shortly.


For those that stumble upon this issue, make sure to use:

const plotly = require('plotly.js/dist/plotly.js');

When requiring plotly. Everything will then work just fine in Electron land.

@rgbkrk rgbkrk changed the title Plotly.js requires gl Plotly.js requires gl in Electron contexts Aug 31, 2016
@rgbkrk
Copy link
Author

rgbkrk commented Aug 31, 2016

Related support for Plotly in nteract: nteract/nteract#662

Possibly related gl error:

screen shot 2016-08-30 at 8 06 41 pm

@etpinard
Copy link
Contributor

etpinard commented Sep 1, 2016

Requiring plotly.js as require('plotly.js/dist/plotly.js') should make this issue disappear.

Is there an electron field we could add to the package.json to tell Electron to use the dist bundle instead of resolving all the src/ requires?

I'm guessing this is because Plotly.js determines that it's running in a node context

The only reason that happens is because of our mapbox-gl dependency. I think this issue should be moved to https://github.com/mapbox/mapbox-gl-js/issues unless there's a way to Electron to use the dist/ bundle.

@rgbkrk
Copy link
Author

rgbkrk commented Sep 1, 2016

Is there an electron field we could add to the package.json to tell Electron to use the dist bundle instead of resolving all the src/ requires?

Sadly, no.

I'll try the dist path, sounds like an awesome option.

@sglyon
Copy link

sglyon commented Sep 1, 2016

@rgbkrk how do you tell that gl isn't working inside electron?

I my Julia package (PlotlyJS.jl) that uses Electron as a display window I am able to make a scattergl trace, for example

EDIT: I see -- just saw your edit :)

@etpinard
Copy link
Contributor

etpinard commented Sep 1, 2016

@spencerlyon2 the gl issue (see -> https://github.com/stackgl/headless-gl ) can be seen by require('plotly.js)of version1.16.0and up which includemapbox-gl(to generatescattermapboxtraces) which itself requires thegl` module.

Our gl2d and gl3d trace types don't require gl.

@rgbkrk
Copy link
Author

rgbkrk commented Sep 1, 2016

Ok great, that dist bundle works perfect for displaying the plot above.

rgbkrk added a commit to rgbkrk/nteract that referenced this issue Sep 1, 2016
@sglyon
Copy link

sglyon commented Sep 1, 2016

@etpinard thanks for the explanation.

I must be getting around it without knowing because I'm running plotly.js 0.16.2 inside electron without any problems -- oh well, don't fix it if is isn't broken!

@rgbkrk
Copy link
Author

rgbkrk commented Sep 1, 2016

I'll have something nice for you soon @spencerlyon2 that you can use to send the plotly JSON spec over to this same component in nteract: nteract/nteract#662

@sglyon
Copy link

sglyon commented Sep 1, 2016

That's great. I really need to take a closer look at nteract.

I've been very happy with the combination of Electron + plotly.js for my Julia package -- it let's me to some really cool streaming things.

rgbkrk added a commit to rgbkrk/nteract that referenced this issue Sep 1, 2016
@etpinard
Copy link
Contributor

etpinard commented Sep 2, 2016

@rgbkrk @spencerlyon2 Ok if I close this issue?

@sglyon
Copy link

sglyon commented Sep 2, 2016 via email

@rgbkrk
Copy link
Author

rgbkrk commented Sep 2, 2016

My second comment in this thread is fixed, not the core bit. We're still stuck with a native dependency on gl because of the npm install.

@etpinard
Copy link
Contributor

etpinard commented Sep 2, 2016

We're still stuck with a native dependency on gl because of the npm install

... in a node.js context that is: which we don't support (at least until jsdom/jsdom#1368 is done - as plotly.js makes use of XMLSerializer).

@rgbkrk
Copy link
Author

rgbkrk commented Sep 2, 2016

I may end up creating a separate package specifically for electron uses that cuts a version based on plotly.js, though I'd rather not manage that and let it get out of sync.

@etpinard
Copy link
Contributor

etpinard commented Sep 2, 2016

Yeah good point about the maintenance burden.

We might end up publishing another package specifically from node contexts (plotly.js-node anyone??) again once jsdom support all the browser things plotly.js uses. But, that said, the main plotly.js package will remain a browser-only package in the foreseeable future.

@rgbkrk
Copy link
Author

rgbkrk commented Sep 2, 2016

Yup, browser only is fine for me (browserifiable/webpackable that is).

@rgbkrk
Copy link
Author

rgbkrk commented Sep 6, 2016

Ok we just tried to switch our build setup to not have any compilers, etc. available and the fact that npm install will try to bring in gl fails on us:

nteract/nteract#687

@rgbkrk
Copy link
Author

rgbkrk commented Sep 9, 2016

What if gl is a dev dependency and if someone is really using this from node, they have to install gl themselves?

@etpinard
Copy link
Contributor

etpinard commented Sep 9, 2016

What if gl is a dev dependency and if someone is really using this from node, they have to install gl themselves?

I'd vote 👎 on that, plotly.js is a browser-only app at the moment (and for the foreseeable future).

I don't see why we should add a dev dependency to make sure that require('plotly.js') works in a node context, even though Plotly.plot() won't work.

@rgbkrk
Copy link
Author

rgbkrk commented Sep 10, 2016

I think something is lost in translation here. I want a browser only app too and gl is not used in the browser version - it is in the node version (and currently in the dependencies).

@rgbkrk
Copy link
Author

rgbkrk commented Sep 10, 2016

Welp, I'm a jerk. It was a peer dependency of a downstream dependency that has been resolved since upgrading. No gl necessary.

Sorry to waste your time @etpinard, thank you for the lovely JS.

@rgbkrk rgbkrk closed this as completed Sep 10, 2016
@FrantisekGazo
Copy link

FrantisekGazo commented Oct 18, 2016

I got the Uncaught Error: Cannot find module 'gl' when trying the plotly.js in Electron app.

I installed version 1.18.1 via npm install plotly.js

only require('plotly.js/dist/plotly.js') works

@etpinard
Copy link
Contributor

@FrantisekGazo

please read #891 (comment)

@rgbkrk
Copy link
Author

rgbkrk commented Oct 19, 2016

I've updated the issue description to include the require into dist so people who end up at this issue see the way to handle it upfront.

@rgbkrk
Copy link
Author

rgbkrk commented Apr 12, 2017

Since our overall bundled electron app was rather big (especially for Hydrogen), @lgeiger made minimal-plotly with only the .min.js build. It would be good for us to use the source distribution as much as possible (to allow for flattened npm dependencies and tree shaking), this is our first pass for now though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants