vmpowerio/chartjs-node

Allow modification of global ChartJS variables

Closed this issue ยท 12 comments

Hi there!

ChartJS has a number of global variables which would be very useful to be able to set so one doesn't have to set it for every chart they have created. It'd be very useful to have an (optional) way of passing these variables down to ChartJS.

Thanks for your time.

Hello!

Maybe we could pass an options object on ChartjsNode constructor and assign it to the Chartjs.defaults.

ex:

let options = {
  defaults:{
    global:{
        hover:{
            mode : 'whateverMode'
            }
        }
    }
};
let chartNode = new ChartjsNode(600, 600, options);

And then just assign it to Chartjs.defaults like this:

 Object.assign(Chartjs.defaults,this._options.defaults);

Something like that? ๐Ÿ˜„

This was one of the reasons I elected to make Chart.js a peerDependency. Since we are using the same module, if you do:

Chartjs = require('chart.js')
Chartjs.defaults.global.hover.mode = 'nearest';

When chartjs-node requires Chart.js -- it should just pick up the changes, since the subsequent require on the module will just return same object. Does that not work out?

Gonna close this one out. @devmattrick go ahead and reopen if you think more needs to be done here.

Sounds good. Wasn't aware that the subsequent require would return the same object. Obviously need to brush up a bit on my Node :P

for me this doesn't work out as when I require chart.js I get ReferenceError: window is not defined
at chart.js/src/core/core.helpers.js:665.

Seems like I need to require chart js within the jsdom env callback where window is available. Unless im doing something wrong?

Also thanks a lot for this as it's making server side rendering of charts soooo much easier.

Thanks for the feedback @leighquince glad to see it's helpful! ๐Ÿ‘

Do you mind pasting a code snippet that reproduces the error you mentioned?

it was just the inclusion of chart js that did it, version 2.5.0

let Chartjs = require('chart.js')

so not actually an issue really with this repo but goes with the conversation about being able to update the globals

Ah - you're right. From my viewpoint this is an issue with this project since you can't load the chartjs module unless you pull in jsdom yourself -- which the point of this package is to avoid that.

I'll take a stab at a PR to take an options field which can set this as originally suggested by @FilipeDominguesGit

I also require access to the global chart instance, I need to change the defaults for the built in scales, however these defaults are not on the global object but need to be changed by calling Chart.scaleService.updateScaleDefaults().

There are also a few other Chart properties that may be useful for other applications, such as Chart.layoutService, Chart.helpers, etc. Maybe providing a callback with access to the global chart would be more flexible?

var chartNode = new ChartjsNode(600, 600, function(Chartjs) {
    Chartjs.defaults.global.hover.mode = 'whatevermode';
});

Then in index.js

class ChartjsNode {
    constructor(width, height, onInit) {
        this._width = width;
        this._height = height;
        this._onInit = onInit;
    }
   ...
   const Chartjs = require('chart.js');
   if(this._onInit) {
        this._onInit(Chartjs );
    }

Of course this would also provide an alternative to the current plugins and charts options...

Yeah seems like a good idea so that this library doesn't have to chase around Chart.js features. I'm not a fan of callbacks in constructors. Instead how about emitting an event.

let chartNode = new ChartjsNode(600, 600);
chartNode.on('beforeDraw', Chartjs => {/**do whatever**/; });
chartNode.drawChart(configuration);

We can make ChartjsNode extend EventEmitter. In the future if anything else (synchronous) needs to be done within the jsdom environment, we can add event hooks.

I think my only concern is that would someone ever want to do something asynchronous within the jsdom environment? My thought is that anything asynchrounous required for the chart can be done before the drawChart call.

Yes the event route seems a better approach. I can't think of anything someone would need to do asynchronously, at least with the ChartJS global so I think the event implementation will be pretty robust that way.

I have implemented this in my fork, along with tests and docs. I will implement and test the usage in my application, it should be ready for a pull request soon.

This feature does also make the plugin and chart configuration options redundant, since they can be done on the ChartJS instance, would you want to mark them as deprecated?

FYI, my pull request for this feature has been merged. Documentation for how to use has been added to the Readme as well.