pablojim / highcharts-ng

AngularJS directive for Highcharts
MIT License
1.73k stars 463 forks source link

using V1.1.0 you cannot use alternate versions/products of Highcharts other than 5.0 with ES6 imports #624

Closed dhodgin closed 6 years ago

dhodgin commented 6 years ago

I was using a traditional form of implementing highcharts-ng and have switched to using webpack to load all my modules.

When using webpack with import syntax in my angular app it becomes impossible to use angular-ng with a version of highcharts other than highcharts 5.x which is bundled with the package now.

Highcharts also has Highstock and Highmaps and those files are included with the bundled dependencies with this project but cannot be used like before when using import syntax

Before, I could load highcharts/highstock.js in my script tag before loading highcharts-ng in a script in my index.html file via bower installs and it would use Highstock with highcharts-ng.

Highstock contains everything Highcharts does plus more for stock charts. So it is effectively both.

However, now with v1.1.0 it includes Highstock 5.x in the project and when you import "highcharts-ng" you end up with Highcharts 5.0 in your bundle.js file.

If you also import "highcharts/highstock" before the highchart-ng you will end up with Highcharts 5.x AND Highcharts 6.x in your bundle.js file and the directive will use 5.0 and you've just bloated your bundle with 2 versions of Highcharts for no reason and 6.x will never be used.

This becomes a big problem because anywhere you try to use a Highstock chart type the page breaks and says Highcharts is not a constructor because its trying to access the Highstock constructor from Highcharts 5.0, bundled with this package.

My solution so far has been to downgrade to v1.0.1 and write my imports in the following way:

import "highcharts/highstock";
import "../../js/highcharts.theme";
import "highcharts-ng";

And then also in order to get the theme to work (because the theme file requires Highcharts be a global, and that global will be global to the bundle function not the window object) you have to modify your webpack config

  plugins: [
    new webpack.ProvidePlugin({
      jQuery: "jquery",
      $: "jquery",
      "window.jQuery": "jquery",
      Highcharts: "highcharts/highstock"
    })
  ]

This tells webpack to inject Highstock anywhere in the bundle it sees that Highcharts global is needed.

Only by adding the plugin was I able to get my theme to continue to work.

I've seen other solutions that involve importing Highcharts from highcharts and then attaching Highcharts to window via window.Highcharts but this ends up attaching Highcharts 6 to window.Highcharts but the directive here uses highcharts 5.x from the bundle webpack creates and you will end up with 2 versions of highcharts being delivered client side.

One on window.Highcharts (v 6.x) and one in the bundle.js file (v5.x included with this package v1.1.0)

So far the leanest solution has been downgrade to highcharts-ng v1.0.1 and add my own Highstock v6 import to my app file followed by the theme, followed by highcharts-ng and then setup the Highcharts global in the webpack.config for the theme.

This way Highcharts only appears once in the bundle.js file and it is version 6.x of Highstock not Highcharts and my theme works.

Is there a proposed way to override the included highcharts v5.x in this package in v1.1.0 with your own version or is this currently a bug?

Should highcharts-ng allow the users to specify which product of Highcharts to use from the dependencies at least? so we can do something like import "highcharts-ng" if we want regular highcharts and import "highcharts-ng/highstock" or import "highcharts-ng/highmaps" if we want Highstock or Highmaps instead.

That will also only allow use of v5 of the product as well not the current 6. Licensing for the product ties you to your specific version and upgrading to 6 if you have a v5 license is technically against the license i believe.

This project really should not be including Highcharts by default as a lot of users won't know they are unintentionally violating the license agreement by using a version or product they haven't paid for necessarily.

pablojim commented 6 years ago

see #623

I propose removing the dependency and documenting this clearly for new users.