Skip to content
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

[FEATURE] Charts as modules #4461

Closed
jeantoledo opened this issue Jul 4, 2017 · 10 comments
Closed

[FEATURE] Charts as modules #4461

jeantoledo opened this issue Jul 4, 2017 · 10 comments

Comments

@jeantoledo
Copy link

I think that a modern chart library should work in the following way:

import PieChart from 'chart/pie';
new PieChart(/necessary stuff/);

Prós:

  • The page only requests js for a specific chart.
  • Better use of single responsibility pattern.
  • Better IntelliSense??
  • ...

What you think guys?

Thanks.

@etimberg
Copy link
Member

etimberg commented Jul 4, 2017

Definitely a good enhancement to consider that makes some things simpler. Would definitely be a breaking change though

@benmccann
Copy link
Contributor

@simonbrunel said in #4303 that he was going to cleanup the modules and switch them to ES6. Would we want to implement something like this as part of that change?

@willgm
Copy link

willgm commented Jul 5, 2017

@etimberg, not necessary a breaking change, this can be completely optional, as it is now a days with https://github.com/ReactiveX/rxjs

@etimberg
Copy link
Member

etimberg commented Jul 5, 2017

The breaking change is that right now we have no concept of a different class for each chart type. I'm wary of breaking things with a change that introduces that

For example, would these return the same classes?

var mychart = new Chart(ctx, {
  type: 'pie'
});
var mychart = new PieChart(ctx, {});

Another potential question: how would we handle mixed chart types?

@willgm
Copy link

willgm commented Jul 5, 2017

I believe the API doesn't need to change in order to have partial imports.

For example, RxJS have more then one usage approach for it, one of then is the follow:

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/of';
import 'rxjs/add/operator/map';

Observable.of(1,2,3).map(x => x + '!!!'); // etc

Chart.js could use a similar approach like:

import { Chart } from 'Chart.js/Chart';
import 'Chart.js/add/pie';

var mychart = new Chart(ctx, {
  type: 'pie'
});

@etimberg
Copy link
Member

etimberg commented Jul 6, 2017

@willgm that style of imports would definitely work well with our code. We could even do the same for importing new scale types (log, time, etc).

@aldo-roman
Copy link

I found a workaround for this issue. The code is on Typescript:

import * as core from 'chart.js/src/core/core.js';
import * as helpers from 'chart.js/src/core/core.helpers';
import * as platform from 'chart.js/src/platforms/platform.js';
import * as canvasHelpers from 'chart.js/src/core/core.canvasHelpers';
import * as element from 'chart.js/src/core/core.element';
import * as plugin from 'chart.js/src/core/core.plugin';
import * as animation from 'chart.js/src/core/core.animation';
import * as controller from 'chart.js/src/core/core.controller';
import * as datasetController from 'chart.js/src/core/core.datasetController';
import * as layoutService from 'chart.js/src/core/core.layoutService';
import * as scaleService from 'chart.js/src/core/core.scaleService';
import * as ticks from 'chart.js/src/core/core.ticks';
import * as scale from 'chart.js/src/core/core.scale';
import * as interaction from 'chart.js/src/core/core.interaction';
import * as tooltip from 'chart.js/src/core/core.tooltip';
import * as arc from 'chart.js/src/elements/element.arc';
import * as line from 'chart.js/src/elements/element.line';
import * as linearbase from 'chart.js/src/scales/scale.linearbase';
import * as category from 'chart.js/src/scales/scale.category';
import * as linear from 'chart.js/src/scales/scale.linear';
import * as doughnut from 'chart.js/src/controllers/controller.doughnut';
import * as Doughnut from 'chart.js/src/charts/Chart.Doughnut';

const Chart = core();
helpers(Chart);
platform(Chart);
canvasHelpers(Chart);
element(Chart);
plugin(Chart);
animation(Chart);
controller(Chart);
datasetController(Chart);
layoutService(Chart);
scaleService(Chart);
ticks(Chart);
scale(Chart);
interaction(Chart);
tooltip(Chart);

arc(Chart);
line(Chart);
linearbase(Chart);
category(Chart);
linear(Chart);
doughnut(Chart);
Doughnut(Chart);

Then:

const doughnutChart = new Chart(ctx, {
      type: 'doughnut',
      data: data,
      options: options
});

ChartJS is on package.json
The current setup only uses Doughnut chart

@benmccann benmccann added this to the Version 3.0 milestone Feb 4, 2018
@backspaces
Copy link

When will 3.0 arrive? Any beta available?

BTW: Here is a related closed issue: #5179

@backspaces
Copy link

backspaces commented Sep 1, 2018

I placed a comment in #5179 (comment) which shows how to build a module from chart.js via a rollup with chart.js as a commonjs module. Let me know if it works for you.

Bottom line: with a simple rollup.config and two plugins, chart.js seems to be converted to an esm.

@benmccann
Copy link
Contributor

I'm closing this in favor of #7371 which has much more information on the current state of things and how it could be implemented. We'd love volunteers to help with it. As much as I'd love to see it, I'm not sure it'll make it into 3.0 without someone stepping up to implement it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants