[BUG] Can't update scale.ticks.* options at runtime since 2.7.0

[VerifiedJustLooking]

If you look at this jsfiddle: http://jsfiddle.net/jmpxgufu/179/

I put together a small bar chart, and on an interval (after 5 seconds), it will update some fontSizes and labels on the y-axis. Yet, the tick fontSize won't update (but everything else will).

I am pointing to https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.0/Chart.bundle.min.js (or if you try to point here: http://www.chartjs.org/dist/2.7.0/Chart.bundle.js, it fails as well, it doesn't matter either way -- here's a jsfiddle using that lib: http://jsfiddle.net/jmpxgufu/183/)

I had stolen this jsfiddle from someone and modified it to fit my needs. Previously they were using 2.4.0, and it worked with that library. Did something break between 2.4.0 and 2.7.0?

[jcopperfield]

It seems that v.2.7.0 uses the ticks.minor and ticks.major options to set the font.
Using myLiveChart.scales['myAxis'].options.ticks.minor.fontSize = 36; in the example will set the fontsize correctly.

[jbgtmartin]

It worked for me thanks a lot! I had the same issue with ticks.fontColor and ticks.display.

The docs tell that if they are not specified ,ticks.minor and ticks.major properties inherit from ticks. It is the case when the chart is initialized, but I suppose that when calling chart.update() it doesn't work anymore because ticks.minor and ticks.major are already filled with the initial values so they don't inherit the new ones and we have to edit them manually.

[jcopperfield]

@simonbrunel I might be mistaken, however it seems that PR #4198, actually fixed this bug by recreating the scales on update.

[simonbrunel]

That would need to be tested but I don't think #4198 fixes that issue: scale options are not generated but fetched from the chart.options (eq. config.options). When the chart is created, the scale class generates the minor and major ticks options from ticks under scale.options.ticks.minor/major. At the next update, chart.options.xAxes[index].ticks === scale.options.ticks and already contains minor/major.

Ideally, scale.options should be renamed to scale._options (private) and should always receive a copy of the user options. That would make easier transforming scale._options in each scale without worrying about breaking the user initial options (which IMO, should never be modified by chart's internals).

Though, I think that would be a breaking change since scale.options is public.

[simonbrunel]

Not tested but what we could do in core/core.scale.js:

Chart.Scale = Element.extend({
  initialize: function() {
     // initialize is called from the Element constructor
     this._update();   
  },

  /**
   * Called by the chart controller on update()
   * @private
   */
  _update: function(options) {
     // Note: keep this.options valid for backward compatibility
    this.options = options;
    this._config = this._configure(options);
  },

  /**
   * @private
   */
  _configure: function(options) {
    // Let's first clone `options` which is probably a reference 
    // on the user options and should not be modified internally
    // https://github.com/chartjs/Chart.js/issues/4896
    var config = helpers.clone(options);
    var ticks = config.ticks;
    
    // ... resolve config.ticks.minor / config.ticks.major ...

    return config;
  }
})

Then replace all uses of scale.options by scale._config.

And in core/core.controller.js:

scale = scales[id];
scale.ctx = me.ctx;   //< not sure why it's needed 
scale.chart = me;     //< not sure why it's needed
scale._update(item.options)
// ...

[jcopperfield]

@simonbrunel Exactly the scale options are fetched from chart.options and then mergeTicksOptions is called to regenerate the minor and major options with the newly set options. I tested the fiddle from this bug before and after PR #4198 and it seems to fix it.
I am not saying that it is the most robust solution, however it seems to work.

[simonbrunel]

I can't get it work with master: https://jsfiddle.net/25q2k47z/ (maybe I misunderstood the original issue). mergeTicksOptions doesn't not regenerate ticks.minor.* and ticks.major.* if they already exist, which is the case when calling chart.update() the second time.

So I'm not sure the following case is resolved:

var chart = new Chart(ctx, {
  options: {
    scales: {
      xAxes: [{
        ticks: {
          fontSize: 42
        }
      }]
    }
  }
});

// ...

chart.options.scales.xAxes[0].ticks.fontSize = 12;
chart.update();

[jcopperfield]

I see what you mean, I tested the original fiddle, which hasn't got the fontSize set in the configuration, so updating works. However in your example it wouldn't.

[jcopperfield]

@simonbrunel I like your proposal to introduce a private configuration variable _config. However wouldn't it make more sense to make that part of Chart and use it internally for all options instead of just for scales?

Also I see we currently have a Chart.config.options and a Chart.options pointing to the same object. What is the reason for having both? Could we destine one for being a clone of the other and use it to hold the current configuration until an update() is called?

[simonbrunel]

The idea behind my proposal is to store an intermediary state (_config) which would be the result of parsing options. It's different from the original options since it could contain new properties calculated from input options, skip deprecated ones, pre-calculated values (optims), etc., so would be very specific and private to each entity (controllers, scales, etc.). So I don't think there is any advantage that all entities work on the same global _config (state) at the chart level, it could actually creates some weird internal dependencies between entities and be hard to maintain.

We can still do a deep copy of chart.(config.)?options into chart._options (private) to prevent
this kind of issue, but I would keep the _configure approach per entities.

Chart.config.options / Chart.options ... What is the reason for having both?

Backward compatibility, we can't change their behavior until v3 (for which I would remove chart.config). Anyway, if we deep copy options, _options should be private and not exposed to the user, so we should not use chart.options for that purpose.