Repository navigation
fix(types): distribute ChartDataset over the chart type - #12342
Open
mrityunjay-pandey wants to merge 1 commit into
Open
mrityunjay-pandey wants to merge 1 commit into
mrityunjay-pandey wants to merge 1 commit into
Conversation
With TypeScript 7, or --stableTypeOrdering on TypeScript 6, the dataset of a single chart type was not assignable to ChartDataset<keyof ChartTypeRegistry>, because the options, plugin options and properties of all chart types were intersected into one type. Make ChartDataset a union of complete per-type datasets instead. Fixes chartjs#12285
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12285
ChartDataset<keyof ChartTypeRegistry>(and thereforeChartDatasetwith its default type argument) isn't assignable from the dataset of a single chart type with TypeScript 7, or with--stableTypeOrderingon TypeScript 6:For a union of chart types, the type was the intersection of three parts: a union of the per-type options, the plugin options of all types, and
ChartDatasetPropertiesof the whole union. TypeScript has to match that intersection against each member of the options union. Whether it found the right member depended on the order of the union, so TS 6 accepted it only with its default ordering, and failed in the full type-test suite even then.This makes
ChartDatasetdistribute overTType.ChartDataset<'line' | 'scatter'>is nowChartDataset<'line'> | ChartDataset<'scatter'>, where each member has the options, plugin options andtypeof a single chart type.TDatakeeps its default for the whole union, so thedataof each member is as wide as before. For a single chart type the result is the same type as before.Tests
New
test/types/chart_dataset_assignability.tsassignsChartDataset<'scatter'>,ChartDataset<'line'>andChartDataset<'bar', number[]>toChartDataset<keyof ChartTypeRegistry>,ChartDatasetandChartDataset[].pnpm lint-types(TypeScript 4.9 from the lockfile) passes.test/typesagainst the builtdisttypes with TypeScript 6.0.3, 6.0.3--stableTypeOrderingand 7.0.2. The repo's tsconfig doesn't load on those versions (baseUrl,importsNotUsedAsValues), so I used a minimal one. Onmasterthe new test fails with all three. With this change it passes, and the rest of the suite reports exactly the same errors as onmaster(they come fromstrictin that minimal config), so no new errors.