search

carlcraig / tc-angular-chartjs

AngularJS directives for Chart.js
http://carlcraig.github.io/tc-angular-chartjs/
Other
233 stars 83 forks source link

Incorrect usage of ng-annotate #14

Closed DrewML closed 9 years ago

DrewML commented 9 years ago

Howdy,

It looks like there was a recent update that implemented ng-annotate in the build process. Unfortunately, with it's current configuration, it's breaking my apps build process (which runs a concatenated file through ng-annotate). Although that's clearly a sign that I should fix that part of my build process, I'm sure fixing this would help others as well :).

The specific issue can be seen here: https://github.com/carlcraig/tc-angular-chartjs/blob/master/src/tc-angular-chartjs.js#L20.

Lines 20-26 explicitly define the "$inject" property of each directive/factory, but that's unnecessary as ng-annotate already handles this. Due to that, every "$inject" property ends up duplicated in dist/tc-angular-chartjs.js.

If you were to run the dist file through ng-annotate again, you'd get the following error:

error: conflicting inject arrays

The simple fix is to remove lines 20-26 from src/tc-angular-chartjs.js.

carlcraig commented 9 years ago

I haven't noticed the problem regarding ng-annotate, although you are correct that the lines 20-26 in src/tc-angular-chartjs.js are unecessary as ng-annotate is adding the $inject statements to the dist files.

Please can you let me know if the problem is still happening after 1.0.9.

Thanks.

DrewML commented 9 years ago

@carlcraig That worked perfectly. Thanks for pushing that so quickly. I appreciate it.