Remove type from the scriptable contexts

[simonbrunel]

Following up on chartjs/chartjs-plugin-datalabels#229 which suggests to add type: 'datalabels' on the plugin context. The initial use case is explained in #8623 but I disagree on the way it should be resolved (see this comment). However this raised a few more concerns about (1) limitations that this type introduces but also (2) how we promote the use of scriptable options to our users.

---

1. ctx.type breaks abstraction of the options. It's suggested to use type (#8626) to verify if the received context is valid. However, it's not possible to check if (ctx.type instanceof 'dataset'), thus it doesn't allow to fallback to a context based logic. For example, the datalabels plugin fallbacks its color option to the root one. Let's say I would like to configure the root color to assign a different color to each data, it will not work because type === datalabels, not data:

// Would receive `type === 'datalabels'` when fallback from `datalabels.color`.
Chart.defaults.color = (ctx) => ctx.type === 'data' ? 
  ctx.dataIndex % 2 ? 'red' : 'green',
  'blue';

Checking for dataIndex instead seems a better choice in most cases:

// The datalabels context is a 'data' compatible context, so it works as expected.
Chart.defaults.color = (ctx) => ctx.dataIndex !== undefined ?
  ctx.dataIndex % 2 ? 'red' : 'green',
  'blue';

---

2. As explained in the same comment, there is no reason to check type === '<plugin_id>' | 'scale' | 'tick' | 'tooltip' in options. Also, recommending users to rely on type sounds like it's fine to put all the logic in "generic" options to distribute values across multiple features:

// WRONG
Chart.defaults.color = (ctx) => {
  return ctx.type === 'datalabels' ? 'red' :
     ctx.type === 'tooltip' ? 'green' :
     ctx.type === 'scale' ? 'blue' : 
     ctx.type === 'tick' ? 'yellow' : 
     'grey';
}

I really think this is an anti-pattern and we should not provide API to do that. Instead, users should use the plugin, scale, etc. options or write generic scriptable logic.

---

I'm probably missing some legit use cases but if not, I would suggest to remove type from core contexts before releasing v3:

• type === 'chart': useless, all contexts should be a chart context
• type === 'dataset': check datasetIndex instead
• type === 'data': check dataIndex instead
• type === 'scale': use the scale options instead
• type === 'tick: use the tick options instead
• type === 'tooltip: use the tooltip options instead

[kurkle]

Having a common and easily distinguishable property on the context helps users understand what is happening in their scriptable options. In Chart.js v2 if you write a handler for backgroundColor in a dataset, it might not be so obvious that the first call is made in the dataset context and subsequent calls in data context (in the case of line based core controllers). They might go on and file a bug, because the dataIndex is sometimes missing.

So the intention of the type is to help users understand what they are dealing with.

I'm not sure where it breaks abstraction in different way that dataIndex, for example, does.

In Chart.js v3 many of the options fall back to options root before defaults are looked up. If a user specifies a function for options.color, it should be easy to determine what is calling it. This is where having a type helps. Also missing the type helps, because its going to be a plugin or extension in that case.

[simonbrunel]

I'm not sure where it breaks abstraction in different way that dataIndex, for example, does.

type === 'datalabels' tests a feature while dataIndex !== undefined tests the shape, so can match different features.

type doesn't support inheritance, thus it's not possible to write generic logic such as if (context instanceof DatasetContext). And this is a limitation in some cases. For example, the datalabels plugin for which the context should be compatible with the dataset and data context, should nicely fallback to the dataset options without having to check if (type === 'data' || type === 'datalabels').

While I understand the need to differentiate dataset and data contexts (same option will receive both), I don't think of a "valid" use case where you need to test type === 'scale' | 'tick' | 'tooltip' | 'datalabels'?

[kurkle]

type doesn't support inheritance

It does not remove the dataIndex or datasetIndex, you are free to keep using those.

if (context instanceof DatasetContext)

The contexts are actually inheriting already, but they are just plain objects currently. So that one could be easily achieved.

I don't think of a "valid" use case where you need to test type === 'scale' | 'tick' | 'tooltip' | 'datalabels'

A theme perhaps?

[simonbrunel]

A theme perhaps?

I don't see why it's a use case, themes would provide defaults at each level (scale / plugins / etc.) without checking 'type'.

Anyway, if type is the way to go for core options, I will not push more to remove it. However, I will wait before adding it to the datalabels context until it's actually needed for a concrete use case.