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

Browser loading bug #592

Closed
rafaelmacario opened this issue May 9, 2017 · 9 comments
Closed

Browser loading bug #592

rafaelmacario opened this issue May 9, 2017 · 9 comments

Comments

@rafaelmacario
Copy link

When using highcharts in browser, the variable module param is becoming undefined and throwing an application error

Link to jsfiddle showing the issue.

https://jsfiddle.net/rafaelmacario/chhcpdu6/3/

Expected Behaviour

When loading the highchart module in browser it must detect the module existence in order to let script continue to load

Current Behaviour

highcharts-ng-js : Line 20

if (window && window.Highcharts) {
    Highcharts = window.Highcharts;
  // The module variable does not exists and the following error is thrown
  } else if (module && module.exports === 'highcharts-ng') {
        Highcharts = require('highcharts');
  }

Error: ReferenceError: module is not defined

Your Environment

  • Version of highcharts-ng used: 1.1.0
  • Browser Name and version: Mozilla Firefox For Ubuntu canonical -1.0 52.0.2 (64-bit)
  • Operating System and version (desktop or mobile): Ubuntu 14.04 LTS
  • Link to your project:
rafaelmacario pushed a commit to rafaelmacario/highcharts-ng that referenced this issue May 9, 2017
@churro-s
Copy link

@pablojim, Will this fix go out with the next release on bower?

@lwallent
Copy link
Contributor

Hi @rafaelmacario and @pablojim

I was about to make a PR when I found this issue - introducing what I think i a bug in it self?

All that was needed to fix the bug metioned in issue #592 was the additional check:

typeof module !== 'undefined' 

The check that was introduced:

(typeof module !== 'undefined' && typeof exports !== 'undefined' && module.exports === exports && module.exports === 'highcharts-ng' ) 

will never be true when combined with the top most check/assignment:

if (typeof module !== 'undefined' && typeof exports !== 'undefined' && module.exports === exports){
  module.exports = 'highcharts-ng';
}

If exports and module.exports are equal but not 'highcharts-ng' then when assigning 'highcharts-ng' to module.exports results in the two no longer being equal!! :-)

Hence the later check for

module.exports === exports && module.exports === 'highcharts-ng'

will always fail??

The real check should be:

(typeof module !== 'undefined' && typeof exports !== 'undefined' &&  module.exports === 'highcharts-ng' )

That is, leaving out the module.exports === exports check

How can this work for anyone? I am a little puzzled!

Please let me know if I am missing something here?

Kind Regards

Lasse

pablojim pushed a commit that referenced this issue Dec 20, 2017
Fixes #592 module declaration
@prols
Copy link

prols commented May 3, 2018

Hello, we would like to use your great library for a project in angularJS. I have the version 1.2.0 retrieved from npm repository. But I am block because of this defect.
Would it be possible to tag a new "minor" release for fixing this blocking defect and thus have it available thorugh npm.

Many thanks for this great work!
Regards,
Philippe Rols.

@blemaire
Copy link

blemaire commented May 4, 2018

@prols you can target a specific commit via npm if that helps

@prols
Copy link

prols commented May 7, 2018

Yes thanks I will do that for now.

@prols
Copy link

prols commented May 7, 2018

@blemaire Hum now I run into the same issue reported here:
#643
As the dist folder has not been published I guess I need to rebuild highcharts-ng locally?

@blemaire
Copy link

blemaire commented May 8, 2018

you will indeed need to build it locally, looking at the PR, npm run build should do the trick

@pablojim
Copy link
Owner

@prols Released now.

@prols
Copy link

prols commented May 14, 2018

@pablojim Great thanks!

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

No branches or pull requests

6 participants