-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Chart.js does not import as es6 module #5179
Comments
@simonbrunel any thoughts? I know this got tested at one point |
We currently use browserify to generate UMD but we didn't upgrade for a long time so maybe there is an easy fix that doesn't break any integration? I don't know from where this documentation about ES6 import compatibility comes from but are UMD builds compatible with es6 import since it doesn't provide a
#4478 is about migrating to rollup but this might not happen until v3 because it could break node.js integrations. |
The huge advantage of converting the code to es6 modules is that then, using rollup and possibly babel (for es5) is that you can then convert to every format. It should not impact your node compatibility because rollup easily converts to commonJS. It also lets you rollup subsets of your project, which support fewer chart types. Can you say just what node problems you have? |
We don't have any problem with node :) We care to not break projects that use Chart.js and I'm pretty sure some people integrated it using I agree that migrating to rollup/es6 modules is a good thing but we first need to refactor/clean our way to use modules and that's what we started with #4478 (we already use rollup in other projects such as Though, I'm not sure UMD builds can be imported via es6 import? That means your way to include Chart.js from our UMD build (https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.1/Chart.js) may not work, even with rollup and a code initially based on es6 modules. |
I hear ya! One solution Three.js used was to keep the old names the same, and using an es6 rollup called three.module.js. The older three.js name was kept for the older code. The source itself obviously changed quite a bit but they did not make the source part of the dist/. So you could do likewise: convert to es6 source files, rollup to a cjs bundle named chart.js. OTOH, waiting for the 3.0 build makes sense in terms of project management. I think I can convert chart.js to modules using this stunt:
so no worries. |
But lets change the docs, others will make the same mistake I did. |
I now seriously need plotting for a project I'm involved in .. and we're es6 module based. Our code is vanilla JS es6 w/ import/export. Our very simple workflow uses rollup to create two bundles: es5/UMD and es6/ESM. We use unpkg to serve both formats:
The latter uses the module: property in package.json. Could you think about creating such a dual-release? I see that you use "require", thus browserify. I know very little about that tooling, but I bet you can find tools that can create ESM from this. If you look at d3, you'll see they create bundles for both that work well on unpkg.com. Admittedly you may prefer to simply convert your source to es6 modules, and then bundle to a UMD compatible with your existing chart.js (which works in both the browser and node) and an ESM that is fine for es6 oriented repos. You'll be surprised how much easier it is to start with your core as ESM and convert to any format you'd like. |
@backspaces I think the issue might be that you're using Chart.js 2.7.1 and looking at the docs for Chart.js 2.7.2. Can you try upgrading to Chart.js 2.7.2? I have a vue.js + webpack app that's using Chart.js via ES6 import as described in the docs and it's working for me:
|
Hi, thanks for the reply. Here's the problm: Your import above is not the es6 module syntax which needs a "path" like ./chart.js. Your workflow is making adjustments to your code, I think. Webpack likely is doing it. I tried changing the html in the OP to 2.7.2 and still receive the error.
.. gives:
And looking at the code, it does not appear to have any es6 export. Using unpkg to view the repo, https://unpkg.com/chart.js@2.7.2/ (dashboard view) Asking on Stackoverflow, I did get an interesting response tho: using a "bare" import worked!
There's a working example: https://stackoverflow.com/questions/48410525/how-do-i-import-chartjs-as-an-es6-module I have no idea how that works! It ends up working like a script tag, putting Chart in window. I only tried chrome. |
The difference could also be that I'm using NPM and you're importing a pre-built script. It'd be really interesting to try NPM without webpack to understand whether webpack is involved. @Jareechang do you have thoughts here? (asking because you documented and might have some insight into how this works) |
hey @benmccann, my changes were related to #3788 as it was breaking the import. Unfortunately, I do not have much insights into how this works 😞. I did also use webpack when I was testing it. However, It does sound like es6 is not supported atm as @simonbrunel has mentioned that the UMD builds do not provide a |
Just a note: You can't create a single file that is both <script>able/require-able(node cjs) and es6 importable. The current favorite approach is to have the src/*.js code written as individual es6 modules, then bundled, w/ Rollup, to a UMD (script/require) and to a separate ESM bundle. Then place a pointer to the ESM in the package.json under the "module:" property. This is even made simpler with unpkg which recognizes the ?module query string as a request for the module, via package.json. It is a shame that es6 modules were so difficult to get started. Huge workflow, tools like browserify, jspm, and webpack, typescript having their own modules for quite a while, and so on. And now it's hard for us to go back to the simplicity that modern browsers have made possible. Sigh. :) |
I am facing the same issue. Now that ES6 imports are natively supported in all major browsers it would be nice to have a solution for Chart.js. |
Docs updated in #5555 |
@benmccann You closed the issue but it seems not fixed. Version 2.7.2 is not providing an ESM version of the library. The original issue is not related to using Webpack only or not. Some packaging methods, such as Polymer 3 apps with I just noticed you tag this issue for milestone Version 2.8. Are you going to provide ESM flavors with NPM packages starting with version 2.8.x? |
We updated the docs, which were incorrect, to clarify that chart.js does not yet have es6 support. I don't think that es6 support will be added until 3.0 |
When will 3.0 be released? Is there a beta I could try? |
There are no plans to start working on 3.0 at the moment |
Just a quick update: I believe a simple rollup, along with a commonJS plugin, produces a usable es6 module. The Rollup looks like:
This is built using the chart.js npm install. You'll also have to node install rollup-plugin-commonjs and rollup-plugin-node-resolve I don't have a test env for this but here's the output: http://backspaces.net/temp/chart.esm.js
If anyone uses this and it works, let us know. We also could just add to the projects workflow if it is considered useful. Here's the SO discussing this: https://stackoverflow.com/questions/52068933/can-rollup-plugins-convert-the-majority-of-legacy-libraries-to-es6-modules/52076713#52076713 |
@simonbrunel Thanks for your answer and pointing the wrapper. I have adapted the rollup configuration to use the original source file: https://github.com/noticeableapp/Chart.js/commit/466fc8087d354dd39aa127f365b5ec86003dc3a6
Yes, that's the idea.
What is proposed is far from perfect but the migration you mention, although fantastic, is a huge work that is open since more than a year. ESM is supported by modern browsers. ESM imports are becoming a first-class solution (e.g. it's really helpful with Polymer 3, lit-html, TypeScript). The changes initially proposed by @backspaces could work for a transition period. Since they are non-breaking, you could even release them as Regarding the NPM package, it seems to include doc, samples and tests. It's not the additional ESM variant of the original source code that will make a big difference in terms of size. |
In order to consider this transition period where we would mix browserify and rollup, I would like to investigate if we couldn't completely switch now to rollup without breaking changes. Here is a wip branch ( @backspaces @lpellegr the current setup generates the ESM builds without moment. I'm not familiar with ES modules "in the browser" and don't know how peer dependencies should be handled: <script type="module">
import chart from './dist/Chart.esm.js';
// throws...
// Uncaught TypeError: Failed to resolve module specifier "moment". Relative references must start with either "/", "./", or "../".
</script> Any idea? |
Moment is now natively in es6, within src/ The repo exports src/ and the package.json has an es6 module header, thus this works: So you're close! |
@backspaces I already figured out that import moment from 'moment'; which fail with:
We will also need to find a way to make |
The build has now been migrated to rollup: #5904 |
Is there a reason why the For reference, I'm using pnpm which keeps good track of these inconsistencies: pnpm/pnpm#1700 |
@wbern that will be the case in the upcoming v2.8.0. |
For the record, chart.js works with pnpm without issues, I had a small mistake in my config (in webpack, symlinks: false doesn't work well with pnpm) |
Per the reference to
Per this update, should we expect to no longer receive the following:
when using Thanks! |
Same here, any updates on this? |
See my latest comment |
Import needs you to use a complete path: So try |
Just a quick note. This: #5179 (comment) still works fine, and we're using it at pika/web FredKSchott/snowpack#62 to look into automatic conversion of CJS/UMD format to es6 modules. Here's the current rollup:
The test repo has these node dependencies:
If you'd like, I can make a github repo of the experiment, but it really is trivial. Here's a link to a pika article if you're curious. |
Is there, btw, a reason this is not already in the current chart.js? Make a rollup, add a "module": property to package.json? That doesn't seem very odd. |
That's really sad. More than a year and a half, suggestions provided, no breaking changes, and we still need to struggle to manage case manually. What a shame. |
I have not yet tried it on chart.js specifically, but per @backspaces' comments above, and feature requests in pika-web, you can now do package install-time conversions of commonjs modules to es-modules using pika-web. It's worth a shot. |
@krumware Thanks but Chart.js is now building with Rollup and adding support for ESM is just a matter of adding a few lines of codes in I ended up by forking the release branch, made the small changes mentioned above: https://github.com/ipregistry/Chart.js/commit/b981ac18715f94253a502efeb852b0c4d6218db9 and published my own package: |
Thanks for the heads-up! great to know there are many options. Hopefully they pull your changes so we can get that esm dist out of the box if you potentially aren't able to maintain the fork long-term. |
If anyone wants to send a PR to improve our rollup & package.json config that'd be great as long as it's backwards compatible with all the documented methods of using Chart.js |
@lpellegr I took a look at your change. It looks like your es6 version bundles moment.js. Was that a conscious decision? I think my initial inclination would have been not to include it |
I hope this pull request is as requested. Due to the old age of this ticket and its lively discussion, being in contrast with the small additions needed to the rollup config, I hoped I didn't overlooked some hidden/deeper problem. At least, I tested the new builds in a pure browser scope, as well as rollup scope (without commonjs support). Both working as expected 😃 |
The instructions on http://www.chartjs.org/docs/latest/getting-started/integration.html show chart.js being imported as an es6 module:
When I use this import statement within a module using chrome latest, it fails:
The error is:
Expected Behavior
I think this is a bug, the docs say it should work. However, looking at the code, I see no es6 export.
Current Behavior
It fails with the above error.
Possible Solution
You might try converting to an es6 build using rollup for all conversions to other modules.
https://medium.com/@backspaces/es6-modules-part-1-migration-strategy-a48de0b7f112
Steps to Reproduce (for bugs)
<script type="module">
Context
I am using chart.js to visualize results of agent based models. The repos are:
Environment
The text was updated successfully, but these errors were encountered: