-
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
Propose a simplified registration process #8419
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,17 @@ var myChart = new Chart(ctx, {...}); | |
|
||
## Bundlers (Webpack, Rollup, etc.) | ||
|
||
Chart.js 3 is tree-shakeable, so it is necessary to import and register the controllers, elements, scales and plugins you are going to use. | ||
To use Chart.js, a simple import is all that is required. | ||
|
||
```javascript | ||
import Chart from 'chart.js'; | ||
var myChart = new Chart(ctx, {...}); | ||
``` | ||
|
||
### Tree Shaking | ||
|
||
Chart.js 3 has optional tree shaking that is enabled via the `chart.js/tree` package. To use this, it is necessary to import and register the controllers, elements, scales and plugins you wish to use. The example below shows all of the available imports and how to register them. | ||
|
||
For all available imports see the example below. | ||
```javascript | ||
import { | ||
Chart, | ||
|
@@ -50,7 +58,7 @@ import { | |
Legend, | ||
Title, | ||
Tooltip | ||
} from 'chart.js'; | ||
} from 'chart.js/tree'; | ||
|
||
Chart.register( | ||
ArcElement, | ||
|
@@ -80,6 +88,18 @@ Chart.register( | |
var myChart = new Chart(ctx, {...}); | ||
``` | ||
|
||
A short registration format is also available. | ||
|
||
```javascript | ||
import { Chart, controllers, elements, plugins, scales } from 'chart.js'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really working? I can't find were you export these groups. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I forgot to delete that part of the docs when I switched around how this worked. |
||
Chart.register(...[ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need to wrap everything in an array: Chart.register(
...Object.values(controllers),
...Object.values(elements),
...Object.values(plugins),
...Object.values(scales),
); |
||
...Object.values(controllers), | ||
...Object.values(elements), | ||
...Object.values(plugins), | ||
...Object.values(scales), | ||
]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could make this even easier by exporting an array containing everything: import { registrables } from 'chart.js';
Chart.register(...registrables); You might have a better name for |
||
``` | ||
|
||
## Require JS | ||
|
||
**Important:** RequireJS [can **not** load CommonJS module as is](https://requirejs.org/docs/commonjs.html#intro), so be sure to require one of the UMD builds instead (i.e. `dist/chart.js`, `dist/chart.min.js`, etc.). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,9 @@ | |
"dist/chunks/*.js", | ||
"dist/chunks/*.d.ts", | ||
"helpers/**/*.js", | ||
"helpers/**/*.d.ts" | ||
"helpers/**/*.d.ts", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of the scope of this PR: could this be moved under a single folder and copied when publishing to npm? For example: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think NPM in that case might consider the package to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These extra packages could even be generated by a script as we did with |
||
"tree/**/*.js", | ||
"tree/**/*.d.ts" | ||
], | ||
"scripts": { | ||
"autobuild": "rollup -c -w", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,18 @@ | ||
// Export all of these as we do in the tree-shakeable version | ||
export * from './controllers'; | ||
export * from './core'; | ||
export * from './elements'; | ||
export * from './platform'; | ||
export * from './plugins'; | ||
export * from './scales'; | ||
|
||
import {Chart} from './core'; | ||
import * as controllers from './controllers'; | ||
import * as elements from './elements'; | ||
import * as plugins from './plugins'; | ||
import * as scales from './scales'; | ||
|
||
Chart.register(controllers, elements, plugins, scales); | ||
|
||
// Since everything is already registered, provide a default Chart export for convenience | ||
export default Chart; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export * from './controllers'; | ||
export * from './core'; | ||
export * from './elements'; | ||
export * from './platform'; | ||
export * from './plugins'; | ||
export * from './scales'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You still need to export those groups: // ...
export * from './plugins';
export * from './scales';
// Exporting groups to simplify the registration.
export * as controllers from './controllers';
export * as elements from './elements';
export * as plugins from './plugins';
export * as scales from './scales'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"name": "chart.js-tree-shaking", | ||
"private": true, | ||
"description": "tree shakeable version of chart.js ESM build", | ||
"main": "tree.js", | ||
"module": "tree.esm.js", | ||
"types": "tree.esm.d.ts" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from '../dist/chart.shakeable.esm'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from '../dist/chart.shakeable.esm'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require('..').tree; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think users expect the default behavior (i.e.
import {...} from 'chart.js'
) to be tree-shakable, which may be the most common use case. It's also the natural method to import from Chart.js and it may be better to keep it that way. As a plugin developers, I would also expect the following snippet to only bringLineElement
and not trigger any auto-registration:It may be better to have a special case for auto-registering instead (e.g.
chart.js/auto
orchart.js/all
).