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

ReferenceError: document is not defined #5243

Closed
Tracked by #15
silviubogan opened this issue Nov 3, 2020 · 17 comments · Fixed by #5388
Closed
Tracked by #15

ReferenceError: document is not defined #5243

silviubogan opened this issue Nov 3, 2020 · 17 comments · Fixed by #5388

Comments

@silviubogan
Copy link

silviubogan commented Nov 3, 2020

This dependency makes it impossible to use plotly.js in the browser because image-size uses fs module which is not available in browsers. I do not know how does react-plotly.js adapt to this dependency but I guess they have this issue too.

Related to: #5119.

Reproducible example

  1. Create an app with create-react-app.
  2. Install dependencies from the screenshot below (you might need to change the React and ReactDOM packages' versions too), including plotly.js@1.57.1 and use it as a dynamic import with @loadable/component.
  3. Run yarn start in the project directory.

Screenshot

image

Expected output

The app runs well without terminal or browser errors.

Actual output

Error inside browser window:

image

Update

The essence of this issue is here: #5243 (comment)

@silviubogan
Copy link
Author

Sorry, I think it is a configuration problem on my side. It seems to work with create-react-app. I am closing this because of this.

@silviubogan
Copy link
Author

I get the same error again and in a similar situation, so I think it is real. This time I have more information. We use plotly.js 1.57.0 and react-plotly.js 2.5.0. The code is here and the situation in images is:

Browser window contents

image

DevTools console

react-plotly.js errors

image

Our code's errors

image
image

plotly.js errors

image

I will try to reproduce this issue in a create-react-app project and come with an update.

Thank you.

@silviubogan silviubogan reopened this Nov 9, 2020
@silviubogan
Copy link
Author

I think that the following error might be related to the same issue:
image

@silviubogan
Copy link
Author

I have tested and the screenshot in my message above is the root cause of this issue.

In chronological order:

  1. I start the server.
  2. I open the browser with the web app URL.
  3. The error in my previous message appears in the terminal and approximatively at the same time in the browser I get this:
    image
  4. Refreshing the page in the browser makes the server skip the error and in the browser I see the first errors in this issue.

@silviubogan silviubogan changed the title image-size dependency makes plotly.js not work in browser through webpack ReferenceError: document is not defined Nov 9, 2020
@archmoj
Copy link
Contributor

archmoj commented Nov 9, 2020

@silviubogan could you investigate if probe-image-size depends on fs? And if it does not, can we replace image-size here?

var sizeOf = require('image-size');

@silviubogan
Copy link
Author

In our code, the issue is that these instructions (just them, as I verified), when not commented out, produce the error related to the fs module, although I was thinking this is impossible since I tested them in a context where they are not used anywhere:

const LoadableBasicPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-basic'),
);
const LoadableCartesianPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-cartesian'),
);
const LoadableGeoPlotly = loadable.lib(() => import('plotly.js/lib/index-geo'));
const LoadableGl3dPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-gl3d'),
);
const LoadableGl2dPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-gl2d'),
);
const LoadableMapboxPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-mapbox'),
);
const LoadableFinancePlotly = loadable.lib(() =>
  import('plotly.js/lib/index-finance'),
);

I still do not understand how loadable.lib works. I thought it lazy loads the libraries, so if none of the consts above are used, as I tested, the error related to fs is not justified.

I start investigating the probe-image-size library now, if nothing else more important intervenes.

Thank you!

@silviubogan
Copy link
Author

The probe-image-size library does not require fs and, they say, finds the length only through HTTP. An example usage is this:

  it('should return content-length', async function () {
    responder = function (req, res) {
      res.writeHead(200, { 'Content-Length': 1234 });
      res.end(GIF1x1);
    };


    let size = await probe(url);


    assert.strictEqual(size.width, 1);
    assert.strictEqual(size.height, 1);
    assert.strictEqual(size.length, 1234);
    assert.strictEqual(size.mime, 'image/gif');
  });

(via this link)

But its internal functions are easy to use and extend. If time allows me to do this, I can try to use it for your use cases.

@nicolaskruchten
Copy link
Member

I could be way off the mark here but the document is not defined error suggests to me that you're trying to use this module in a NodeJS context rather than a browser context, which is not really a supported use-case at all... Is that indeed what you're trying to do?

@silviubogan
Copy link
Author

I could be way off the mark here but the document is not defined error suggests to me that you're trying to use this module in a NodeJS context rather than a browser context, which is not really a supported use-case at all... Is that indeed what you're trying to do?

Yes, more precisely we use razzle with SSR. I hope that SSR is supported.

Thank you.

@nicolaskruchten
Copy link
Member

Unfortunately, pure-Javascript SSR is not officially supported at this time and I'm not aware of anyone having successfully done it unofficially, due to this exact issue. Plotly.js relies extensively on browser APIs which are generally not available or hard to emulate on the server short of running a full in memory browser like we do in the Kaleido project.

@silviubogan
Copy link
Author

PTAL and help me solve this issue. Is it an issue in @loadable/component?

In our code, the issue is that these instructions (just them, as I verified), when not commented out, produce the error related to the fs module, although I was thinking this is impossible since I tested them in a context where they are not used anywhere:

const LoadableBasicPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-basic'),
);
const LoadableCartesianPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-cartesian'),
);
const LoadableGeoPlotly = loadable.lib(() => import('plotly.js/lib/index-geo'));
const LoadableGl3dPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-gl3d'),
);
const LoadableGl2dPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-gl2d'),
);
const LoadableMapboxPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-mapbox'),
);
const LoadableFinancePlotly = loadable.lib(() =>
  import('plotly.js/lib/index-finance'),
);

I still do not understand how loadable.lib works. I thought it lazy loads the libraries, so if none of the consts above are used, as I tested, the error related to fs is not justified.

I start investigating the probe-image-size library now, if nothing else more important intervenes.

Thank you!

@silviubogan
Copy link
Author

silviubogan commented Nov 24, 2020

It seems that after commenting all the other loadable.lib calls in the message above, from these remaining two:

const LoadableBasicPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-basic'),
);
const LoadableCartesianPlotly = loadable.lib(() =>
  import('plotly.js/lib/index-cartesian'),
);

the first one alone does not produce the error with fs, but when uncommenting the second one, the error is there. What could be the issue? Is it a Loadable issue?

Thank you.

@archmoj
Copy link
Contributor

archmoj commented Jan 8, 2021

@silviubogan would you mind testing if the issue was resolved after #5388?

@silviubogan
Copy link
Author

@archmoj Thank you for the notification. I am pleased to know you have worked on this. I will try to find time in the following week.

@archmoj
Copy link
Contributor

archmoj commented Jan 9, 2021

See #5377 (comment)

@silviubogan
Copy link
Author

@archmoj The error related to the module image-size is gone (with the latest commit in plotly.js#master) in the project I work on, although I did not test inside CodeSandbox. Is it possible to tell when will the change be included in a GitHub release? Thank you!

@archmoj
Copy link
Contributor

archmoj commented Jan 15, 2021

@archmoj The error related to the module image-size is gone (with the latest commit in plotly.js#master) in the project I work on, although I did not test inside CodeSandbox. Is it possible to tell when will the change be included in a GitHub release? Thank you!

Thanks for testing this out.
We are working on a major release now that involves days of QA.
So v2 might be out around Jan 25, I hope.

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

Successfully merging a pull request may close this issue.

3 participants