-
Notifications
You must be signed in to change notification settings - Fork 21
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
Unable to es6 import plugin #2
Comments
@powerbuoy I always assumed that this plugin would be loaded via a |
@simonbrunel Thanks for the quick reply. It's so strange that it actually works, regardless of the errors thrown. Aurelia has its own bundling process so it may actually have to do with that but I'm not sure. I tried just running:
On https://esnextb.in/ and that (unfortunately, for me :P) works error free. If I reverse the order of the imports I get the same error as I do locally however, so for some reason Aurelia's bundler doesn't work the same as the one on esnextb.in. In my Aurelia app I don't actually even have to import your plugin to get the error, adding it to my dependencies is enough. Unless you're an Aurelia pro I guess you know about as much as I do about the problem though. |
I don't know Aurelia, so can't really help on that and I'm not sure why the plugin still work even with the thrown error (since it still needs to access |
Yes it's puzzling. I've now tried with a few other ChartJS plugins (annotations, zoom and draggable) and none work. In fact when trying to include them the entire build fails, whereas with yours at least the code is built and runs as expected it's just that I get that error in the console. There are so many module loaders and ways to do these things nowadays that I'm not surprised things don't always work as expected, but I find it a little hard to believe I'm the first person trying to import a ChartJS plugin using an ES6 |
The issue is pretty simple to be solved. The library should expose a UMD compatible format, that way the consumer can define when the execution has to happen. By having the build artifact wrapped inside UMD, we can as well declare dependencies, respectively ChartJS, and as such leverage the ES6 import syntax as well. Over at the Aurelia CLI your aurelia.json deps would then look like:
|
Thanks @zewa666, it may be interesting to also comment on chartjs/Chart.js#4160 since it can be useful for plugin maintainers? |
@powerbuoy can you pull @zewa666 branch from #3 and confirm that it fixes your issue with Aurelia? |
@simonbrunel yep could be ... as you said lets @powerbuoy first test the example and then we can see how to generalize this for other plugin maintainers. For your example it was pretty easy since you had a nice infrastructure with Gulp in place, for others it might be more work to propose how to properly add UMD support by themselves. |
@simonbrunel @zewa666 Thanks for your efforts guys! I might be doing something wrong, but here are the steps I've taken:
I've noticed that with Aurelia CLI's build system, a package mustn't contain a "." or it fails to import, that's why I keep renaming ChartJS like that. I'd love to understand why I still need the It also seems the PR is missing the |
Yes, to test the PR, you need to run |
@powerbuoy also please disregard |
I see. Ok I've I've also changed my bundle file to point to the new
(Note that the path shouldn't contain With this in place the Aurelia build now fails, like other ChartJS plugins I've tried it now looks for the plugin's dependencies inside my project root instead of in node_modules. Note that this happens with and without the |
yep the path should not contain the file itself but only the folder. If you still experience issues, can you create a minimal example replicating the issue and upload it to a github library, so I can take a look at it and help you out perhaps. |
Tbh it sounds like you guys know plenty more about all these module loaders so I wouldn't be surprised if it's my lack of experience with them that's making me fail. But like I mentioned before, I use plenty of other node modules and they all work without issue (just add the package name to Setting up a brand new Aurelia project is actually quite simple if you would like to test this locally:
|
Seems we commented pretty much at the same time :) I've tried with this as well:
And I still get the same error about Note that including ChartJS itself has never been an issue though. |
Please try to change the chartjs name to "chart.js" and also use the same name in the deps property. |
Unfortunately that doesn't work with Aurelia CLI; aurelia/cli#349 |
UMD is setup with dependencies over the I don't know if the "dot in name" is a limitation of Aurelia (and so should be fixed on their side) or, more generally, an issue with module loaders (e.g. can't find any restriction in the node guide, except that a name must not begin with a dot). |
I see. It wouldn't surprise me if this is an Aurelia issue tbh. Renaming packages like I do above has worked for me in the past though (with jump.js). But I guess if I install another package that depends on the renamed package this might happen? |
Ok some progress :) If I change:
To:
It works :) So I guess I'll need to reopen aurelia/cli#349 and take it from there. And I guess this PR works perfectly for build systems that don't have the "." bug. Thanks a lot for your help guys, let's hope the Aurelia team is as helpful :D |
So I'm still not sure why you experience the dot issue, but as you said that is a Aurelia CLI specific issue and not part of this. The library is named Regarding that @powerbuoy I'd be really glad if you could create a minimal example to reproduce the issue and upload it somewhere. I tried setting up an example with the Aurelia CLI including the dot in the name and everything worked. So having your example would clarify whether I did something wrong or its really an issue. |
Alright I've set up a brand new Aurelia CLI project and done nothing but add chart.js to it. If you add "chart.js" to the dependencies array inside aurelia.json you should see the same issue I am when trying to run Edit: Btw, I never meant the PR should change to "chartjs". I realise this is no longer ChartJS's problem but rather a problem with Aurelia's build system. I'm surprised it works for you though, I'm running the latest version of au-cli (0.28.0) |
Your aurelia.json should have this at the end of the dependencies section:
and the package needs to be installed with my package.json deps section then looks like follows:
and everything works as expected for me. Using v 0.28.0 as well |
Oh man I never even considered I could keep using the "chart.js" name so long as I supplied the path manually. Especially since the creator of Aurelia told me not to use a period in the name in the referenced Aurelia CLI issue. All works perfect now! Thanks so much for your help guys :) |
hehe I guess that was a bit of a misunderstanding from Rob. The path should not end with Alright, glad we've been able to clarify that, and sorry @simonbrunel for abusing your repos issue for that :) Give me a ping once you've decided how to include the PR and we can chime in on the referred issue to aid other plugin maintainers with an example @powerbuoy do not forget to update the stackoverflow issue btw :) |
Fixed in d22f5a0, @powerbuoy can you confirm that the plugin now correctly loads in Aurelia when using dependencies: [
{
"name": "chart.js",
"path": "../node_modules/chart.js/dist",
"main": "Chart.min"
},
"chartjs-plugin-deferred"
] |
Can confirm it works, and I'm using exactly the same Awesome work 👍 |
Released in v0.3.0 |
I'm using ChartJS in my Aurelia app and it works fine. I wanted to add the deferred plugin so I've npm installed it and added the import like so:
The package imports properly but the code throws errors:
Uncaught TypeError: Cannot read property 'helpers' of undefined
.My guess is it's because the plugin is trying to access the
Chart
object which I assume isn't global?(The first few lines of code try to do the following):
I'm honestly a little new to using
import
(as opposed to just stacking<script>
elements) so I might be doing something wrong, but I'd love to know what?Edit:After further testing the plugin actually seems to work properly, but I still get that error in the console which is really annoying. Any idea what could cause it?
The text was updated successfully, but these errors were encountered: