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

Native ES6 modules vs. the bundle #3518

Closed
oldrich-svec opened this issue Feb 6, 2019 · 41 comments · Fixed by #5708 or Sage-Bionetworks/Synapse-React-Client#1453
Closed

Native ES6 modules vs. the bundle #3518

oldrich-svec opened this issue Feb 6, 2019 · 41 comments · Fixed by #5708 or Sage-Bionetworks/Synapse-React-Client#1453
Labels
bug something broken

Comments

@oldrich-svec
Copy link

oldrich-svec commented Feb 6, 2019

When plotly.js is loaded in Chrome by the native ES6 import, the loading crashes with Cannot read property 'document' of undefined at line 18178, i.e. at var d3_document = this.document;

The problem is that the bundler assumes that when a function is executed without any this, the this will automatically be set to window. This is not the case with ES6 modules, where this will be undefined.

I fixed it by changing line 27724 from }(); to }.apply(self); but this is just a hack :)

@etpinard
Copy link
Contributor

etpinard commented Feb 6, 2019

Thanks for writing in.

When plotly.js is loaded in Chrome by the native ES6 import,

Can you share a reproducible example of how you're going about doing this. Thank you!

@oldrich-svec
Copy link
Author

oldrich-svec commented Feb 7, 2019

Try:

<!doctype html>
<html lang=en>
<head>
<meta charset=utf-8>
</head>
<body>
<script type="module" src="https://cdn.plot.ly/plotly-latest.min.js"></script>
</body>
</html>

or alternatively:

<script type="module">
    import 'https://cdn.plot.ly/plotly-latest.min.js'
</script>

@alexcjohnson
Copy link
Collaborator

In a codepen: https://codepen.io/alexcjohnson/pen/aXEbOa?editors=1011

That particular error is with d3v3 - seems like v4 fixes it so we should get it when we leapfrog to v5. Will be interesting to see if there's anything else in our code that uses this to mean the window (or anything else that's incompatible with ES6 modules... I'm not very familiar) but it doesn't seem like there's much point trying before the d3 upgrade.

@etpinard etpinard added the bug something broken label Feb 7, 2019
@etpinard etpinard added this to the v2.0.0 milestone Feb 7, 2019
@jmsuresh
Copy link

jmsuresh commented May 6, 2019

@oldrich-svec

I fixed it by changing line 27724 from }(); to }.apply(self); but this is just a hack :)

Can you please provide which particular line had to be changed. I am stuck with this for the past two days (tried many things with babel options - nothing worked)

@leolorenzoluis
Copy link

Any fix for this?

@LRagji
Copy link

LRagji commented Jun 14, 2019

Fix for version 1.48.2; the change is at line number 32481 '/node_modules/plotly.js/dist/plotly.js'

@wendelstam
Copy link

Anybody know if there is a issue for this filed in the d3 project?

@oldrich-s
Copy link

oldrich-s commented Aug 1, 2019

@jmsuresh In this file:

https://cdnjs.cloudflare.com/ajax/libs/plotly.js/1.49.1/plotly.js

search for define(d3). You will find:

if (typeof define === "function" && define.amd) this.d3 = d3, define(d3); else if (typeof module === "object" && module.exports) module.exports = d3; else this.d3 = d3;
}();

Change }(); to }.apply(self);

@etpinard
Copy link
Contributor

From #4130 @Arjun-Shinojiya wrote:

I am trying to integrate plotly.js in my Angular 8 project. I successfully integrated it and tested it in local ,that time it is working fine without any error. But when I check it in live server ,that time I am getting cannot read property 'document ' of undefined error.. I also similar issue blog in this repository.`.I wanted to try this hack
changing line 27724 from }(); to }.apply(self); but this is just a hack :)
But I dont know in which module file I can get this line.Currently none of the plotly.js file have 27724 lines

Maybe someone here can help.

@Arjun-Shinojiya
Copy link

Arjun-Shinojiya commented Aug 15, 2019

When plotly.js is loaded in Chrome by the native ES6 import, the loading crashes with Cannot read property 'document' of undefined at line 18178, i.e. at var d3_document = this.document;

The problem is that the bundler assumes that when a function is executed without any this, the this will automatically be set to window. This is not the case with ES6 modules, where this will be undefined.

I fixed it by changing line 27724 from }(); to }.apply(self); but this is just a hack :)

@oldrich-svec can you please tell the exact location of the file which you mentioned in this questions last line? .I am facing the same issue.

@oldrich-s
Copy link

see #3518 (comment)

@Arjun-Shinojiya
Copy link

see #3518 (comment)

So I should use plotly.js via CDN in angular 8 right?

@oldrich-s
Copy link

@Arjun-Shinojiya it is up to you how you use plotly.js ;). The comment just shows you what line you want to edit.

@TravisLRiffle
Copy link

Is editing the line of code the only solution? What happens when I want to upgrade to a new version of plotly? I have to edit it every time?

@meriturva
Copy link

@TravisLRiffle yes you have to reapply your hack every time! For me, it is not a solution for enterprise applications.

@maxmeyer
Copy link

After fixing this, I got a d3_xhr-not found-error although it's defined

@AlanJMac
Copy link

AlanJMac commented Oct 3, 2019

Alternative workaround for Angular 8 upgraders: For those of us coming here having encountered this issue when upgrading to Angular 8, an alternative workaround solution to editing the plotly.js file is to not use the new "Differential Loading by Default" as mentioned in this comment on angular-plotly.js: plotly/angular-plotly.js#75 (comment)

I could solve the issue forcing the typescript to be generated as "target": "es5" making it not use the <script src="..." type="module" />, thus both plotly.js and angular code is served by "legacy" way.
Please check this link for more informations: https://blog.angular.io/version-8-of-angular-smaller-bundles-cli-apis-and-alignment-with-the-ecosystem-af0261112a27

So changing the following property in the tsconfig.json file hides the issue:
{ "compilerOptions": { "target": "es5" } }

@michielkikkert
Copy link

Is there a 'real' solutions for this available? I'd like to use Plotly for an Angular 8 project - but reverting to es5 is not really an option (it will break for Angular 9 I think). I was so hoping to use plotly as a Highchart replacement - but no dice I'm afraid.

@kundralaci
Copy link

Just tried with the current latest version (1.52.2), and it's still broken.
Angular 9 has also been released, so it's now the two latest versions that are affected. :(

We have a large Angular-based application, that we cannot update because of this issue. :(

@AlanJMac
Copy link

Just tried with the current latest version (1.52.2), and it's still broken.
Angular 9 has also been released, so it's now the two latest versions that are affected. :(

We have a large Angular-based application, that we cannot update because of this issue. :(

First, I would recommend taking a look at https://github.com/plotly/angular-plotly.js as that is apparently working with Angular 8.x+.

Otherwise, if it is an option for your project, I would recommend loading plotly.js via the CDN.
I have upgraded to Angular 9 by lazy-loading the minified plotly.js this way and it is working very well.
The main bundle is then also much smaller :-)
(I'm no longer using the workaround I mentioned in my previous comment)

@wendelstam
Copy link

wendelstam commented Feb 13, 2020

I managed to get it working in ng9 with ivy by including plotly.min.js as a regular script reference on the page and then PlotlyModule.plotlyjs = (<any>window).Plotly; with the PlotlyModule. I could not get PlotlyViaWindowModule, it compiled but got some invalid plotly version error.

Used angular.json to copy it to assets from the node_modules.
"assets": ["src/assets", { "glob": "plotly.min.js", "input": "./node_modules/plotly.js/dist/", "output": "./" }],

@TrevorKarjanis
Copy link

TrevorKarjanis commented Aug 16, 2020

@fourgates I am using Plotly.js 1.54.5 with angular-plotly.js 2.0.0 in Angular 10 successfully. The property read issue only occurred when plotly.js/dist/plotly.js was imported into a shared module or a file imported into a shared module. I resolved the issue by ensuring the following two things:

  1. The PlotlyModule is imported only in leaf modules. Shared modules require leaf modules to import the dependency if they use angular-plotly.js or plotly.js.
  2. Import the PlotlyService and use PlotlyService.getPlotly to access the global Plotly.js API. I only experience problems with shared files, but I use this practice everywhere.

@archmoj
Copy link
Contributor

archmoj commented Aug 31, 2020

@oldrich-svec have you tried the above solution?
Wondering should we keep this issue open?
If so could you please provide an update using latest plotly.js version?
Thanks.

@oldrich-svec
Copy link
Author

I have just tried to execute v1.54.7 code as described in #3518 (comment) and yes it is still failing with

Uncaught TypeError: Cannot read property 'document' of undefined
    at plotly-latest.min.js:20
    at Object.164 (plotly-latest.min.js:20)
    at a (plotly-latest.min.js:7)
    at plotly-latest.min.js:7
    at Object.728.../constants/numerical (plotly-latest.min.js:61)
    at a (plotly-latest.min.js:7)
    at plotly-latest.min.js:7
    at Object.1.../src/lib (plotly-latest.min.js:7)
    at a (plotly-latest.min.js:7)
    at plotly-latest.min.js:7

I do not use Angular so I did not test the above solution.

If I am not mistaken, all the solutions are either

  • to hack the plotly file as I did
  • or to load plotly as normal script and not ES6 module

@jackparmer
Copy link
Contributor

This issue has been tagged with NEEDS SPON$OR

A community PR for this feature would certainly be welcome, but our experience is deeper features like this are difficult to complete without the Plotly maintainers leading the effort.

Sponsorship range: $10k-$15k

What Sponsorship includes:

  • Completion of this feature to the Sponsor's satisfaction, in a manner coherent with the rest of the Plotly.js library and API
  • Tests for this feature
  • Long-term support (continued support of this feature in the latest version of Plotly.js)
  • Documentation at plotly.com/javascript
  • Possibility of integrating this feature with Plotly Graphing Libraries (Python, R, F#, Julia, MATLAB, etc)
  • Possibility of integrating this feature with Dash
  • Feature announcement on community.plotly.com with shout out to Sponsor (or can remain anonymous)
  • Gratification of advancing the world's most downloaded, interactive scientific graphing libraries (>50M downloads across supported languages)

Please include the link to this issue when contacting us to discuss.

@xuke444
Copy link

xuke444 commented Nov 1, 2020

Looks like plotly needs to upgrade d3 to at least v4

@tony
Copy link

tony commented Jan 2, 2021

(This isn't just an angular issue, all the codesandbox URLs on https://github.com/plotly/react-plotly.js are having this now)

Could anyone here get a working version of typescript + plotly.js on code sandbox?

All versions of those are failing as well. (for the sake of demos on this project and others)

@Eghizio
Copy link

Eghizio commented Jan 23, 2021

(This isn't just an angular issue, all the codesandbox URLs on https://github.com/plotly/react-plotly.js are having this now)

Could anyone here get a working version of typescript + plotly.js on code sandbox?

All versions of those are failing as well. (for the sake of demos on this project and others)

@tony same issue here. Cannot use react-plotly.js on Codesandbox and Stackblitz, neither JavaScript or TypeScript.
It seems it is a plotly.js issue.

@davidhaley
Copy link

OP's fix works for me. Should there be a PR for this?

@acmiyaguchi
Copy link

I haven't been able to use the precompiled bundles in a Svelte+rollup application that I've been building, but I have been able to use the ES6 module e.g. plotly.js/lib/core. Here's a solution that works for me in rollup.config.js:

import replace from "@rollup/plugin-replace";

export default {
    ....
    plugins([
      replace({
        "}()": "this.d3 = d3;\n}.apply(self);",
        delimiters: ["this.d3 = d3;\n", ";"],
      }),
    ...
    ])
}

I can't use plotly.js-dist because I get an issue about process.

error
Assigning to rvalue (15657:0) in node_modules\plotly.js-basic-dist\plotly-basic.js
15655: };
15656: process.title = 'browser';
15657: process.browser = true;
       ^
15658: process.env = {};
15659: process.argv = [];

@juliusbierk
Copy link

Related: https://community.plotly.com/t/issue-in-electron-built-version/31303/5 (although this fix of marking as external does not work for me).

@juliusbierk
Copy link

juliusbierk commented Feb 16, 2021

Solution for vue-cli-service (using https://www.npmjs.com/package/string-replace-webpack-plugin)

const StringReplacePlugin = require("string-replace-webpack-plugin");

module.exports = {
  chainWebpack: config => {
    config.module
      .rule('plotly')
      .test(/plotly\.js$/)
      .use('stringreplace')
        .loader(StringReplacePlugin.replace({
                replacements: [
                    {
                        pattern: /module.exports = d3; else this.d3 = d3;\n}\(\);/,
                        replacement: function() { return 'module.exports = d3; else this.d3 = d3;\n}.apply(self);' }
                    }
                ]}))
        .end()
  }
};

@tomsej
Copy link

tomsej commented May 5, 2021

Seems it is not going to be fixed even in v.2.0.0 :(. So if anybody needs to fix this in minified version, look for:

"object"==typeof e&&e.exports?e.exports=t:this.d3=t}()

and replace it with:

"object"==typeof e&&e.exports?e.exports=t:this.d3=t}.apply(self)

@nicolaskruchten
Copy link
Contributor

In v2 we are no longer exporting d3 so we will have a path towards upgrading it to a version that doesn't suffer from this problem, but indeed, this current problem doesn't automatically go away in v2.

@archmoj archmoj removed this from the v2.0.0 milestone May 11, 2021
plankter added a commit to BodenmillerGroup/histocat-web that referenced this issue May 14, 2021
@VinodLouis
Copy link

VinodLouis commented May 21, 2021

If anyone wants to implement the change in node_module and use a much more radical approach, leverage the patch-package utility.

In package.json scripts, one can use it following command to patch the node_modules

"postinstall": "./node_modules/.bin/patch-package"

The patches for plotly 1.58.4

diff --git a/node_modules/plotly.js/dist/plotly-with-meta.js b/node_modules/plotly.js/dist/plotly-with-meta.js
index 1aaac5a..cb3ecfe 100644
--- a/node_modules/plotly.js/dist/plotly-with-meta.js
+++ b/node_modules/plotly.js/dist/plotly-with-meta.js
@@ -35864,7 +35864,7 @@ Object.defineProperty(exports, '__esModule', { value: true });
     return request.responseXML;
   });
   if (typeof define === "function" && define.amd) this.d3 = d3, define(d3); else if (typeof module === "object" && module.exports) module.exports = d3; else this.d3 = d3;
-}();
+}.apply(self);
 },{}],170:[function(_dereq_,module,exports){
 module.exports = function () {
     for (var i = 0; i < arguments.length; i++) {
diff --git a/node_modules/plotly.js/dist/plotly.js b/node_modules/plotly.js/dist/plotly.js
index 365230c..6268511 100644
--- a/node_modules/plotly.js/dist/plotly.js
+++ b/node_modules/plotly.js/dist/plotly.js
@@ -35864,7 +35864,7 @@ Object.defineProperty(exports, '__esModule', { value: true });
     return request.responseXML;
   });
   if (typeof define === "function" && define.amd) this.d3 = d3, define(d3); else if (typeof module === "object" && module.exports) module.exports = d3; else this.d3 = d3;
-}();
+}.apply(self);
 },{}],170:[function(_dereq_,module,exports){
 module.exports = function () {
     for (var i = 0; i < arguments.length; i++) {

The patches for d3 3.5.17

diff --git a/node_modules/d3/d3.js b/node_modules/d3/d3.js
index aded45c..d5b3cad 100644
--- a/node_modules/d3/d3.js
+++ b/node_modules/d3/d3.js
@@ -9551,4 +9551,4 @@
     return request.responseXML;
   });
   if (typeof define === "function" && define.amd) this.d3 = d3, define(d3); else if (typeof module === "object" && module.exports) module.exports = d3; else this.d3 = d3;
-}();
\ No newline at end of file
+}.apply(self);

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

Successfully merging a pull request may close this issue.