-
Notifications
You must be signed in to change notification settings - Fork 12k
Description
When I updated from Chart.js 3.0.0 beta 7 to RC 3, I started getting type errors similar to the following in my code:
src/legendUtils.ts:39:20 - error TS2532: Object is possibly 'undefined'.
39 const original = defaults.plugins.legend.labels.generateLabels;
~~~~~~~~~~~~~~~~~~~~~~~
I saw that the type definition for plugins was changed in 49b0916 to use Partial.
If I'm understanding correctly:
- If a plugin isn't registered, then its corresponding
defaults.pluginsproperty is undefined / not set. - If a plugin is registered, then its corresponding
defaults.pluginsproperty is always set. - If a plugin is in use, then setting its corresponding
defaults.pluginsproperty to undefined stands a good chance of breaking things.
Letting defaults.plugins be Partial accurately reflects item 1, but it means that any code that references it has to do non-nullable assertions (which goes against common ESLint configurations) or optional chaining operators every time it accesses it (even though item 2 says that it's guaranteed to be set, as long as register is correctly called), and it allows you to set the defaults.plugins to undefined (which would break item 3).
Weighing all of that, I think it makes the most sense to mark defaults.plugins properties as required (in spite of item 1). To do this, I believe you could change the type of PluginChartOptions itself to not use Partial (since most places that reference it are already using DeepPartial), or you could change Defaults to extend Required<PluginChartOptions<ChartType>>.
Feedback / disagreement / etc. welcome. Thank you.