RequireJS and chart.js module name

[potatopeelings]

With the current AMD module ID of chart.js, RequireJS will not use any paths configured in requirejs.config. It will instead attempt to load chart.js from <root>/chart.js (where root is where the HTML page which calls RequireJS is located)

Here is the folder structure to replicate the problem

index.html
main.js
app.js
lib/
    chart.js
    angular-chart.js
    angular.js
    require.js

index.html

<script data-main="main" src="lib/require.js"></script>

main.js

require.config({
    paths: {
        'angular': 'lib/angular',
        'angular-chart': 'lib/angular-chart',
        'chart.js': 'lib/Chart',
    },
    deps: ['app']
});

app.js

 define(['angular-chart'], function (angularChart) {
 });

RequireJS will attempt to load the chart.js AMD module from ./chart.js instead of ./lib/chart.js. This is because of

If a module ID has one of the following characteristics, the ID will not be passed through the "baseUrl + paths" configuration, and just be treated like a regular URL that is relative to the document:
• Ends in ".js".
• Starts with a "/".
• Contains an URL protocol, like "http:" or "https:".

See http://requirejs.org/docs/api.html#jsfiles.

The fix would be change the Require.JS module ID of chart.js to something that doesn't end in .js and doesn't fall in any of the 3 buckets above.

This seems to be similar to twitter/typeahead.js#1211.

Also related - this SO question http://stackoverflow.com/questions/31288001 (there's a workaround in this, but I think it's a bit hacky)

[jantimon]

From the requireJS usage documentation http://requirejs.org/docs/api.html#jsfiles

requirejs.config({
    //By default load any module IDs from js/lib
    baseUrl: 'js/lib',
    //except, if the module ID starts with "app",
    //load it from the js/app directory. paths
    //config is relative to the baseUrl, and
    //never includes a ".js" extension since
    //the paths config could be for a directory.
    paths: {
        app: '../app'
    }
});

would rename chart.js to chart help without breaking backwards compatibility?

[vaneshmali]

So, Is there need to change the code of angular-chart.js (If I am making chart.js to chartjs or chart some other name that not ends with .js)?

[vaneshmali]

I tried to change module ID name from chart.js to chartjs and for the same doing change in angular-chart code, NOT WORKING.

[potatopeelings]

@vaneshmali - there's the AMD module name chart.js and also the angular module name (which also happens to be chart.js).

Changing the AMD module name (should) fix the issue. This part...

// AMD. Register as an anonymous module.
define(['angular', 'chart.js'], factory);

[potatopeelings]

@jantimon

would rename chart.js to chart help without breaking backwards compatibility?

The fix won't be backward compatible because the AMD module name change would mean that the requires.config and any require.defines that refer to the chart.js module name will need to be changed.

[jantimon]

So the main problem is that the module name of Chart.js ends with .js right?
This is hard to fix in this module..

[vaneshmali]

There should be a standard and easy way to FIX it. Hope it will...!!! Waiting for the same.

[potatopeelings]

@jantimon - correct, and just to clarify - the problem is with the AMD module name. The Angular module name can remain as chart.js. Does that change the impact / feasibility?

It's a problem only for folks who use RequireJS by the way, but it's tricky to get it working in an actual production folder structure if you have RequireJS. Maybe a note in the documentation to change the AMD module name in case of issues would help (unless it's too hacky)

Cheers!

[jirihelmich]

+1

From requirejs docs:

There may be times when you do want to reference a script directly and not conform to the "baseUrl + paths" rules for finding it. If a module ID has one of the following characteristics, the ID will not be passed through the "baseUrl + paths" configuration, and just be treated like a regular URL that is relative to the document:

- Ends in ".js".
- Starts with a "/".
- Contains an URL protocol, like "http:" or "https:".

This way, I cannot use this module as a bower component, because I need to change the dependency name and that change would be lost on next update of the component.

If you change it to, for instance, chartjs, a user of the component is able to override that value in his config using paths:

{
    paths: {
            'chartjs':  '../bower_components/Chart.js/Chart.min'
    }
}

However, this is ignored because of charts.js ID:

{
    paths: {
            'chart.js': './bower_components/Chart.js/Chart.min'
    }
}

[vaneshmali]

I have/had changed moduleID name from chart.js to chartjs. Nothing helped. Also tried: changing the angular-chart code as per moduleID name. Nothing worked.
Finally, switched to Google-charts!

[jirihelmich]

The you did something wrong. I changed it to chartjs, added path config as
described in my first comment and everything works as it should.
Thu 16 Jul 2015 v 06:31 odesílatel Vanesh Mali notifications@github.com
napsal:

I have/had changed noduleID name from chart.js to chartjs. Nothing helped.
Also tried: changing the angular-chart code as per moduleID name. Nothing
worked.
Finally, switched to Google-charts!

—
Reply to this email directly or view it on GitHub
#147 (comment)
.

[jirihelmich]

So if I understand correctly, this will be "solved" just by adding a note to docs?

[potatopeelings]

@jirihelmich - that's probably my fault in proposing that (in the light of the comment that it was hard to fix). I would definitely love a fix than a workaround in the docs. Cheers!

[jirihelmich]

Yes, me as well. Otherwise I need to not to use this plugin as a bower component.

What prevents us to modify to module ID for AMD and use chartjs instead of chart.js.

The fact that it's a kind of breaking change (which, in fact, corrects a huge broken thing :-) )?

Because what we need for Require is simply this: e14eca2

[ced59ric]

Is it possible to solve it and release a new version ? Because that's a pretty annoying problem.. Thanks !

[olee]

Please solve that quickly (change priority to high)!
How can you mess up the module name like that in the first place where the RequireJS docs explicitly state to not include the extension for obvious reasons.
Took me one hour to figure out why the load process kept and kept failing.
Changing define(['angular', 'chart.js'], factory); to define(['angular', 'chart'], factory); would fix this problem and there should also be minimal problems with fixing something that is already broken to begin with. All applications that already placed the chart.js in the root because of this problem will still load properly if it is changed to chart because RequireJS will convert it into chat.js.

[jantimon]

This wont work for webpack which is able to properly detect chart.js in the modules folder.
The core issue is that the name of chart.js ends with .js and this cannot be solved in this module.

[olee]

Actually this needs to be solved in this module and nowhere else.
The docs clearly state that there one should not add the extension to the module name.
And because this library violates this definition without any proper reason, we get these problems.

[jantimon]

Sorry but this is just wrong - there was no extension added. The module name is chart.js including the .js.

See here: https://github.com/nnnick/Chart.js/blob/master/package.json#L2

[ced59ric]

So, how do you explain it works with the version 0.6.0 ?

[jtblin]

@olee No need to be so rude.

---

All,

As @jantimon explained Chat.js is the name of the module, as per https://github.com/nnnick/Chart.js/blob/master/package.json#L2.

Before 0.7.0 there was no support for RequireJS so I'm kind of puzzled of how it could work better but apparently removing the .js in the AMD definition would work better but would break for webpack. @jantimon can you confirm that?

Would removing the entire AMD definition work better then (if it works in 0.0.6, it's that it isn't really needed) or fixing the AMD definition and just reversing the order of AMD and CommonJS for webpack compatibility?

[jirihelmich]

Yep, removing the definition would help as well.
Sat 8 Aug 2015 v 08:45 odesílatel Jerome Touffe-Blin <
notifications@github.com> napsal:

@olee https://github.com/olee No need to be so rude.

All,

As @jantimon https://github.com/jantimon explained Chat.js is the name
of the module, as per
https://github.com/nnnick/Chart.js/blob/master/package.json#L2.

Before 0.7.0 there was no support for RequireJS so I'm kind of puzzled of
how it could work better but apparently removing the .js in the AMD
definition would work better but would break for webpack. @jantimon
https://github.com/jantimon can you confirm that?

Would removing the entire AMD definition work better then (if it works in
0.0.6, it's that it isn't really needed)?

—
Reply to this email directly or view it on GitHub
#147 (comment)
.

[landscapeme]

Same problem here.. I tried the 0.6.0 and it works. Is it possible to remove the AMD stuff to makes it work again ? It's not needed, I can confirm that.

[jtblin]

@jantimon could you please confirm the question in the comment above as you implemented this feature originally? Can I just remove .js from the AMD definition and move the AMD definition below the CommonJS for webpack compatibility?

I'd rather do that then removing it entirely but will have to do if no better choice.

[jantimon]

Webpack does some special optimizations for the AMD part - this would be broken but beside of that it should work

[jtblin]

Changed the module ID for chart.js as otherwise it doesn't work. Had to add a shim for angular in the require config to make it work in the example: https://github.com/jtblin/angular-chart.js/blob/master/examples/amd.js.

I don't think it's a breaking change in the sense that it didn't work before so will just push a patch version,

[jirihelmich]

Great! Thanks!
Sun 30 Aug 2015 v 12:57 odesílatel Jerome Touffe-Blin <
notifications@github.com> napsal:

Closed #147 #147.

—
Reply to this email directly or view it on GitHub
#147 (comment).

[jantimon]

Now it is broken for webpack users

[jirihelmich]

Broken in a way you cannot compensate with some webpack configuration? (Sorry, don't know webpack config). Because that was exactly the problem with require - there was NO way how to use it with require.

[jtblin]

@jantimon with a simple example, I can't even get webpack to find angularchart.js lib, it doesn't look like it's reading the AMD config at all as interestingly it chokes on angular and angular-chart, not chart. I'll open a separate issue to track.

Was it working before?

[jantimon]

I am on vacation right now but I'll set up a demo project soon :)

[jirihelmich]

From what I just saw, I don't think that this is already released into bower.

[vquigley]

Is this still an issue? I'm not able to use charts with requirejs.

[xavisavvy]

I think the major confusion here is the angular name vs the requirejs name. I sat and beat my head against the keyboard for 3 hours trying to figure out why things werent working when i finally realized the following worked:

app.settings.json (we have a utility client that we merge the settings with on bootstrap, this will likely be in a different file for a more vanilla require.js)

...
"require": {
    "paths": {
        "app.module": "app.module",
        "app.require": "app.require",
        "app.routes": "app.routes",
        "app.servers": "app.servers",
        "app.settings": "app.settings",
        "app.settings.urls": "app.settings.urls",            
        "angular.bootstrap": "../build/vendor/angular-bootstrap/ui-bootstrap.min",            
        "app.httpBackend": "./modules/httpBackend/app.httpBackend",
        "chart": "../build/vendor/Chart.js/Chart",
        "angular-chart": "../build/vendor/angular-chart/angular-chart"
    },
    "shim": {
        "angular-chart":{
            "deps": ["angular", "chart"]
        }
    }
},
...

app.require.js

// Specify requirejs app dependencies
define([
// From xyz-utility-client
    'bootstrap',  
    'jquery',
    'jquery.ui',
    'jquery.jeditable',
    'jquery.mask',
    // Local to app
    'app.httpBackend',
    'services/appServices',
    'directives/appDirectives',
    'controllers/appControllers',
    'ngCookies',
    'angular-chart'
], function () {
return {
    // Specify app angular module dependencies
    ngDependencies: [
        'ng',             // Angular
        'ngAnimate',
        'ngResource',     // Angular $resource
        'ngSanitize',
        'mgcrea.ngStrap',   // bundle files angular-strap.js, angular-strap.tpl.js
        'ui.router',
        'cgBusy',
        'app.httpBackend',
        'appServices',
        'appDirectives',
        'appControllers',
        'ngCookies',
        'chart.js' // THIS IS THE IMPORTANT PART I MISSED EVERY TIME ITS NOT NAMED ANGULAR-CHART IN ANGULAR.
    ]
};
});