-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
module is not compatible with rollup.js bundling and tree-shaking. #48
Comments
Should also say that I'm using Ionic 2, was working fine before Angular 2 moved to modules. |
In Ionic2 RC0, you need to make some changes in the rollup config for angular2-chartist to work: In node_modules@ionic\app-scripts\config\rolllup.config.js, change the commonjs() line to : You can see reference to this issue here |
Is there anything I can do here to avoid extra configuration? Or is this On Thu, Oct 13, 2016, 12:52 PM Maxence Lehuraux [email protected]
|
It can be fixed on your side from what I read. It seems to be more of an incompatibility with how your component is distributed and how rollup would like it to be distributed. Here's what @danbucholtz says in valor-software/ng2-charts#440:
|
Thanks, yes it also looks like they're going to default to webpack soon instead of rollup. Renaming issue to reflect rollup incompatibility. |
So my final fix/workaround for this was to install the latest
As an aside, I did get a weird problem where my line chart had a filled black area, even with showArea set to false, worked around with with css. Assume this is more a problem/change with base chartist library. |
I'm currently working on a fix for this to hopefully avoid any extra On Tue, Oct 18, 2016 at 12:21 AM Christopher Cook [email protected]
|
Thanks @Paradox41! Thanks, |
I released this as 0.10.0. Hopefully there are no breaking changes, please let me know if this fixes the problems people are seeing and if the extra configuration can be removed. |
Thanks, it might be closer but it's not working for me, unless I'm using it wrong? I'm assuming I just import the Module and the rest of my app can then use
Error message:
Any chance you could use the quick ionic 2 starter (http://ionicframework.com/docs/v2/getting-started/installation/) to try to display a chart for yourself? I appreciate that might take time but I also note that the example code in the readme is based on an old angular 2 rc pre-NgModule and that could be a stumbling block for people trying to use this component/module. perhaps solving this would help resolve that too. |
I wonder if this has to do with Typings. Currently I can't specify more than one typings location (or concat typings). Looking at the path to the file What version are you on? |
I've tried angular2-chartist 0.9.5 and 0.10.0. No old typings or typings.json, just using npm types for chartist. I've given up for now as I've spent an impressive number of hours trying to get chartist working in some form and the people I'm doing the work for won't be impressed :) Can you confirm that this currently works in regular Angular 2.0.x? |
Yes, I can run it locally with Angular 2. I'll have to spend more time On Wed, Oct 19, 2016, 9:35 AM Christopher Cook [email protected]
|
Unfortunately I got 0.9.5 working, then saw your update and applied it. Should have committed my changes! Now can't seem to get back to a working version despite trying all sorts of variations. Have currently reverted to using Chartist directly in my own component but the Ionic component lifecycle is causing issues when page navigation takes place, which is something your component seemed to handle fine (which is why I'd eventually like to get it working again). |
I'll do my best to fix it tonight. Sorry for the trouble. On Wed, Oct 19, 2016 at 9:48 AM Christopher Cook [email protected]
|
No worries, good luck! |
Closes #48 Signed-off-by: Will Soto <[email protected]>
@webprofusion-chrisc I just tested again locally and everything works as expected, the module is being exported correctly. I refactored a bit so it is one file now, but that shouldn't change anything for consumers to be honest. If you continue to have issues, it could be an ionic quirk that I have to look into more. Thanks. |
@webprofusion-chrisc update on this. I did as you asked and tested with the ionic 2 starter using the updated build, everything worked as expected. I get some I am gonna go ahead and cut a release for this. |
I just tested and I still had to modify the rollup.config.js to have this in commonjs():
Otherwise, still works good |
Hmm, when I tried it
If I just do
That's with or without a custom rollup config entry. Not sure whats different. |
Unfortunately, I am not sure what is happening. I was able to successfully compile an ionic app using the module. Is it possible there is some configuration you have somewhere that is throwing it off? |
Yes, it's possible! This is a different machine (and app) from the one I had problems with though, so I'll try again on the other machine tomorrow. |
Alas I just can't get it working. Sorry, just one more check: can you (or anyone else) confirm that |
Ok, I've narrowed it down - For angulars AOT compilation to work the package needs a |
I think ionic build is deprecated. |
@mlehuraux-maf ionic build isn't deprecated at all, it runs the same build step (ionic-app-scripts). The remaining issue is compatibility with angular 2 AOT step using ngc, although you have indicated you can get it to build. I can assure you that the process is not working for me, it does work with npm run build --dev but I'm guessing that skips AOT. |
I'm trying to reproduce the findings here. I've setup two identical Ionic projects with a minimal chartist setup one with the current apps-scripts using rollup and one with the beta apps-scripts using webpack. Rollup: Anything screwed up in my simplified setup?` Versions: home.ts
app.modules.ts
|
@jensstruemper while 'ionic serve' (with Rollup) doesn't run the ngc compiler step, it does bundle with Rollup which needs the type declarations in order to identify which exports can be pruned from the output. You might be able to fix that by adding a reference to the chartist module in the declarations.d.ts. I'm currently using the dev webpack version of the build scripts (with build --dev specified in the package.json build step). Attempting to build in production (so with AOT being applied and ngc being used instead of tsc) also results in the Module Not Exported error. I believe this is because AOT requires libraries to have a .metdata.json file alongside their .d.ts file, although my attempts to generate that myself didn't work. |
@webprofusion-chrisc thanks for sharing some background on the ionic run / build process! I added On webpack: If I add the "--dev" argument ionic run android would stall at "build dev finished" so no apk is generated. Would be happy to continue with JiT for now but as it appears I can't even get the most basic setup running at the moment. |
With the introduction of app-scripts 0.39 I can use "--dev" argument now (no more freezing) so it is working with JiT. I tried to generate the metadata.json using ngc too. Although the compilation finished successfully no metadata.json was generated. Hope the docs for angular2 module creators / maintainers get better soon. |
The current sample code does not cover the inclusion of the module/component via
@NgModule
app import.This causes a problem for me as trying to import the ChartistModule (rather than importing the component at a page level as it used to be) results in the error Unexpected value 'ChartistComponent' exported by the module 'ChartistModule' , suggesting that I'm importing it incorrectly.
The text was updated successfully, but these errors were encountered: