chore!: enable noUncheckedIndexedAccess

[nickofthyme]

Summary

Enables noUncheckedIndexedAccess across all code.

Details

Enabling the Typescript strict mode does not check for array index access (microsoft/TypeScript#39560) and assumes assessing a value at an array index produces the array/tuple type. However this can be deceiving and lead to errors in the code if this array access returns undefined.

Enabling this across the /src creates a consistent and more accurate type checking. This does require a bit more type guards to ensure the value is defined.

Note on tsconfig.json structures

So typescript/VScode changed how they respect nested tsconfig.json files. In the past, VScode would only look to the top-level tsconfig.json and ignore all other named-varients (i.e. tsconfig.lint.json) or nested tsconfig.json files (i.e. storybook/tsconfig.json).

Now this has changed, to where VScode will apply type checking using the nearest parent directory containing a tsconfig.json file. But the bad thing about this is that this assumes that tsconfig.json includes all child directories which may not be and often is not true! Thus making type checking a much bigger pain for it to be consistent between the IDE plugins and the tsc cli checks.

Ideally, we could apply certain compilerOptions to different directories and/or files much like eslint, because enabling noUncheckedIndexedAccess in test files is unnecessary and a little annoying.

To further complicate things, even if this did work, typescript automatically includes transitive imports thus requiring type checking on src code using different compilerOptions leading to conflicting linting errors like...

../packages/charts/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts:93:9 - error TS2783: 'nice' is specified more than once, so this usage will be overwritten.

93         nice: false,
           ~~~~~~~~~~~

  ../packages/charts/src/chart_types/xy_chart/state/selectors/get_api_scale_configs.ts:96:9
    96         ...scaleConfigsByGroupId[groupId],
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    This spread always overwrites this property.

Here spreading the value of scaleConfigsByGroupId[groupId] In one case is assumed to always be defined thus thinks that nice is always overridden, and the other case it's possibly undefined.

So with that said, the best solution is just to apply the same compilerOptions throughout the code and avoid nested overly-constrained tsconfig.json files, in favor of tsconfig.<something>.json.

The next idea is to allow forced unwrapping array access (i.e. someThing[0]!.someProperty) everywhere except /src. But there are no linting rules to enforce this, the only semi-related rule is @typescript-eslint/no-unnecessary-type-assertion

Issues

close #1757

Checklist

• The proper chart type label has been added (e.g. :xy, :partition)
• All related issues have been linked (i.e. closes #123, fixes #123)
• New public API exports have been added to packages/charts/src/index.ts
• Unit tests have been added or updated to match the most common scenarios

[nickofthyme]

@markov00 Before I fix all the errors from this change, I wanted to get your take on this change? I have run into this more and more and I feel not enabling this makes the type checking very loose.

Example

A great example of this is fetching a single spec like this...

elastic-charts/packages/charts/src/state/utils.ts

Lines 21 to 33 in 16ce203

	/**
	* Returns first matching spec
	* @internal
	*/
	export function getSpecFromStore<U extends Spec>(
	specs: SpecList,
	chartType: ChartType,
	specType: string,
	): U | undefined {
	return (
	(Object.values(specs).find((spec) => spec.chartType === chartType && spec.specType === specType) as U) ?? undefined
	);
	}

Currently if we call this function like...

const spec: HeatmapSpec = getSpecFromStore<HeatmapSpec>(specs, ChartType.Heatmap, SpecType.Series);

Typescript does NOT throw any errors and is completely fine with spec being HeatmapSpec type completely ignoring the possibility of undefined, even with the explicit return type!

The downsides I see to this are...

• Accessing non-explicit tuple values from arrays. We use this paradigm in quite a few places, especially partition charts.
• It assumes static array values can be undefined, which is understandable cuz they could change. This is not too common with our code.

[markov00]

@nickofthyme I totally agree with these changes, I see the benefit and I think is desirable, go on with it please

[markov00]

Just code review (I skipped the changes in Storybook because it looks relatively safe).

[markov00]

Good to merge when CI is green.
One thing: for safety reasons what if we merge this as a breaking change and generate a release just before merging this? it could be easy to track/rollback in case of failures