[BUG] Barchart default scale option 'offset: true' not working properly

[sonowz]

Expected Behavior

Default value of offset option (for scale) when chart is barchart should be true. See documentation

Current Behavior

It only defaults to true, iff it is the first scale (in scales list), and its type is 'category' (not 'time').
Otherwise it defaults to false if user doesn't specify the option. (If specified, it works as expected.)
Example:
First 'category' scale WORKS (JSFiddle)
Second 'category' scale FAILS (JSFiddle)
First 'time' scale FAILS (JSFiddle)

Also, I found out this bug produces "non-stack bars drawn off canvas" bug #5312 .
Therefore fixing this issue will also fix that.

Possible Solution

This looks like nasty bug. Long source code explanation below.

Notice that in source code, there are default options for "chart type" (config.type e.g. 'bar', 'doughnut'), and also for "scale type" (config.option.scales[i].type e.g. 'category', 'time', 'linear').
"User config" is what we put in chart constructor new Chart(ctx, config).

And currently there are only two locations in which default value is written:

• 'bar' chart type default as true code
• Any scale type default as false code

It looks weird, and this is causing the bug.

Explanation

Let's think of case when we are merging "chart type default"(defaults[config.type]) and "user config"( config.options):

Chart.js/src/core/core.controller.js

Lines 38 to 41 in a0a195f

	config.options = helpers.configMerge(
	defaults.global,
	defaults[config.type],
	config.options || {});

See the following code called inside the function helpers.scaleMerge():

Chart.js/src/core/core.helpers.js

Lines 52 to 59 in a0a195f

	if (!target[key][i].type || (scale.type && scale.type !== target[key][i].type)) {
	// new/untyped scale or type changed: let's apply the new defaults
	// then merge source scale to correctly overwrite the defaults.
	helpers.merge(target[key][i], [scaleService.getScaleDefaults(type), scale]);
	} else {
	// scales type are the same
	helpers.merge(target[key][i], scale);
	}

If "user config" has only one scale and its type is 'category' (or has no scale option), the flow goes to else statement.
Otherwise, it goes into if statement, and "user config" is merged into "scale type default" (which has offset: false).

Ta-da! So now "user config" has offset: false (even if you didn't specify), before actually merging with "chart type default",
which has offset: true (if chart type is 'bar'), but overwritten to false.

The pain is that this is an expected behavior in general if you think of other cases.
I couldn't think of any elegant solution (which is why I posted as an issue instead of PR) but two dirty workarounds:

• Exceptional precedence: Manually add offset: true to all scales in "user config" (config.options) if its chart type is 'bar'
• Only one default: Make default offset false even if chart type is 'bar' (which I think requires major version up, and worsen UX)

I think we can come up with much better solution than these. Please help.

[benmccann]

Yes, this is a frustrating issue. I'm going to close this as a duplicate of #5191

Please let me know if you think these are actually different issues

Pull requests welcome if you'd like to take a stab at fixing it