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

Problem with Gulp "Cannot find module" #93

Open
kristian2x opened this issue Aug 3, 2018 · 19 comments
Open

Problem with Gulp "Cannot find module" #93

kristian2x opened this issue Aug 3, 2018 · 19 comments

Comments

@kristian2x
Copy link

I am trying to use react plotly but when gulp is bundling all files I get the following error:
Error: Cannot find module './core' from 'C:\Users\...\node_modules\plotly.js\dist'

I saw there have been problems when using webpack but I haven't found anything gulp related.

I followed this guide to setup and import Plotly: https://plot.ly/javascript/react/

Any help?

@nicolaskruchten
Copy link
Contributor

Can you share a link to a repo or example that shows this problem? As it stands right now I don't have enough information to help you :)

@kristian2x
Copy link
Author

I am using this gulpfile: https://gist.github.com/kristian2x/5a590446c6f94751a8ff8e5e8f1193b7
and I am using the example from the link to import (I have installed the packages and double checked that they are present in the node_modules folder):

import React from 'react';
import Plot from 'react-plotly.js';

class App extends React.Component {
  render() {
    return (
      <Plot
        data={[
          {
            x: [1, 2, 3],
            y: [2, 6, 3],
            type: 'scatter',
            mode: 'lines+points',
            marker: {color: 'red'},
          },
          {type: 'bar', x: [1, 2, 3], y: [2, 5, 3]},
        ]}
        layout={ {width: 320, height: 240, title: 'A Fancy Plot'} }
      />
    );
  }
}

The complete error I get is:

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: Cannot find module './core' from 'C:\Users\...\node_modules\plotly.js\dist'
    at C:\Users\...\node_modules\browser-resolve\node_modules\resolve\lib\async.js:55:21
    at load (C:\Users\...\node_modules\browser-resolve\node_modules\resolve\lib\async.js:69:43)
    at onex (C:\Users\...\node_modules\browser-resolve\node_modules\resolve\lib\async.js:92:31)
    at C:\Users\...\node_modules\browser-resolve\node_modules\resolve\lib\async.js:22:47
    at FSReqWrap.oncomplete (fs.js:152:21)

Let me know if you need anything else.

@m90
Copy link

m90 commented Aug 4, 2018

I'm running into the same issue trying to import the module in a Browserify bundle. It's trying to lookup inexistent files in the dist folder.

@m90
Copy link

m90 commented Aug 4, 2018

Interestingly it bundles as expected when I create the component myself from plotly by doing:

import createPlotlyComponent from 'react-plotly.js/factory';
import Plotly from 'plotly.js';

const Plot = createPlotlyComponent(Plotly);

@kristian2x
Copy link
Author

@m90 Thanks for your help but unfortunately I am getting the same error even when I try it with createPlotlyComponent

@oracleruiz
Copy link

Hi devs,

The problem is that plotly.js pkg name changed. Look how to install the pkg. https://www.npmjs.com/package/plotly.js

Do this
npm install plotly.js-dist
and then
change the file /node_modules/react-plotly.js/react-plotly.js at line 11 to:
var _plotly = require('plotly.js-dist/plotly');

@nicolaskruchten Please include this fix.

Thanks

@m90
Copy link

m90 commented Aug 14, 2018

I looked further into this and it seems to be an issue with plotly.js itself and the way it is being bundled. Browserify is not able to use the plotly.js/dist/plotly entrypoint for whatever reason, also when used without react-plotly. The plotly.js-dist also has issues with Browserify.

@m90
Copy link

m90 commented Aug 14, 2018

I filed an issue in the main repo: plotly/plotly.js#2902

@nicolaskruchten
Copy link
Contributor

OK so it seems from the reports here like there's an issue when using Browserify and importing directly from react-plotly.js without going through the factory.

To me, the factory is the best solution in almost all cases because it allows you full control over the plotly.js bundle that's being used, so you don't have to pay the download-size cost of the full bundle if you just want bar charts. I would say that my official recommendation is to load up a version of plotly.js that works for your build system and pass it to the factory.

If we have to choose a set of defaults that works with either Browserify or Webpack, I would be inclined to go the Webpack route, based on my understanding of the relative use-share of these tools in the React community.

That said, if merging #94 (basically having the defaults load from plotly.js-dist instead of plotly.js/dist) solves this issue with Browserify, then I'm happy to do so, because the -dist bundle is much lighter-weight :)

@nicolaskruchten
Copy link
Contributor

this should now be resolved with plotly.js 1.40.0. Can someone confirm please?

@kristian2x
Copy link
Author

I can confirm this is working for me now. Thank you!

@m90
Copy link

m90 commented Aug 17, 2018

I can also confirm that it works as expected when bundling with Browserify when you depend on plotly.js@^1.40.0 🥇

Should this:

"plotly.js": ">1.34.0",
be bumped now as well? Or are you planning to use -dist now?

@kristian2x
Copy link
Author

@nicolaskruchten while it's working my bundled JS file now has increased by around 10mbs! @m90 Do you have the same issue?

@m90
Copy link

m90 commented Aug 21, 2018

@kristian2x My bundle has always been humongous, so I did not pay too much attention (yet). The version of plotly that react-plotly pulls in is 5.7MB non-minified (and hasn't really changed between 1.39.4 and 1.40.0), which makes me wonder how you accumulate 10MB.

How did you import react-plotly before the increase? Did you use the factory approach?

@kristian2x
Copy link
Author

@m90 I couldn't import it at all before the fix. You can have a look at my gulp file here: https://gist.github.com/kristian2x/5a590446c6f94751a8ff8e5e8f1193b7 (vendors.js is the file that increases around 10mbs and I don't know why it's that much). In react I import it the same way as stated in the docs (not the factory approach): https://plot.ly/javascript/react/

@m90
Copy link

m90 commented Aug 21, 2018

@kristian2x I didn't dig too deep into it, but from a quick glance at your gulpfile I would assume that you are including plotly twice because of the following:

  • react-plotly.js pulls in the dist version of plotly all by itself
  • then you also specify plotly as a top-level dependency and include it in the vendors bundle. As this does not specify an entry point, it will pull in ./lib/index.js which is probably going to recreate the entire dist bundle a second time.

I would assume that simply removing plotly.js from your dependencies should work, but then again: this is a very superficial analysis. Maybe this is better debugged in another context than this issue in case it does not work.

@kristian2x
Copy link
Author

@m90 I really appreciate your effort Fredrik unfortunately though this did not solve the issue. I will have a closer look at my gulpfile and I guess this is a problem on my side. Thanks

@m90
Copy link

m90 commented Aug 21, 2018

FWIW, I just double checked on the project that had me run into the original issue, and if I use tinyify to optimize my bundle (containing mostly React, ReactDOM, plotly and react-plotly) I will end up at ~950kb gzipped (3.1MB uglified), so if you are currently adding 10MB on top, there must be something in your build setup that secretly sneaks in something that shouldn't be there.

@m90
Copy link

m90 commented Aug 21, 2018

@kristian2x Btw, a cool tool for debugging size issues w/ Browserify is discify, maybe that gives you some insight on where the bloat is coming from in the first place.

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

Successfully merging a pull request may close this issue.

4 participants