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

Echarts expected on 'window' #1933

Open
vhdirk opened this issue May 6, 2022 · 6 comments
Open

Echarts expected on 'window' #1933

vhdirk opened this issue May 6, 2022 · 6 comments
Assignees

Comments

@vhdirk
Copy link
Contributor

vhdirk commented May 6, 2022

Describe the bug
The function registerThemes, but also pretty much every other class/function, expects the echarts object to be available in the global scope. At first I though this was only for lazily loaded modules, but I can reproduce it for the root component as well.

Stacktrace:

Uncaught ReferenceError: echarts is not defined
    registerTheme covalent-echarts-base.mjs:4321
    registerDefaultThemes covalent-echarts-base.mjs:4328
    7958 covalent-echarts-base.mjs:4819
    Webpack 13
covalent-echarts-base.mjs:4321:4

To Reproduce
Steps to reproduce the behavior:

  1. Clone and run this project: https://github.com/vhdirk/covalent-echarts-test
  2. Click the link on the page
  3. See error in console

Expected behavior
Echarts should just load. At this point, it is just not usable.

Desktop:
any

Smartphone:
any

Additional context
It should be possible to import the echarts library using import * as echarts from 'echarts'; . However, I think the echarts object should be a singleton, so it should be provided in a service (that is provededIn: 'root'). I think the best option would be to clone what they did in: https://www.npmjs.com/package/ngx-echarts#treeshaking-custom-build

I think each covalent echarts submodule could register what it uses on the global 'echarts' object?

@owilliams320
Copy link
Collaborator

@vhdirk as part of the updates to latest angular and echarts. the recommendation is that you pull in echarts using the script array within your angular.json. This is due to the way echarts is packaged as a CommonJs Module.

example from our app

 "scripts": [
     "node_modules/echarts/dist/echarts.js"
 ]

@vhdirk
Copy link
Contributor Author

vhdirk commented May 11, 2022

@owilliams320 Thanks for the reply. I realize that it can indeed work that way. However, we're then completely missing out on the tree-shaking capabilities, which would be quite sad.

@owilliams320
Copy link
Collaborator

Yes that would be an issue needed to be raised with echarts project. The problem is their package is only commonjs

@vhdirk
Copy link
Contributor Author

vhdirk commented May 12, 2022

I'll create an issue with them then. What package type would it need to be?

@JoshSchoen
Copy link
Contributor

@owilliams320 I'm pretty sure v5 introduced modules for better tree shaking. I know I seen it in echart for react and they have an example on their site. @vhdirk any interest in taking a look and contributing?

@vhdirk
Copy link
Contributor Author

vhdirk commented May 12, 2022

@JoshSchoen I mentioned in the gitter channel that I'd be willing to contribute, yes.

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

3 participants