Change and reorder the types definitions

chartjs / chartjs-plugin-annotation

Annotation plugin for Chart.js

MIT License

609 stars 329 forks source link

Change and reorder the types definitions #759

Closed stockiNail closed 2 years ago

stockiNail commented 2 years ago

This PR is changing the types definitions in order to:

have all types on top of the file
remove AnnotationCoordinates because adds the options in CoreAnnotationOptions
remove AnnotationPluginCommonOptions and changes common option defintion to accept a types annotation options
reorder the options by their name, sorting alphabetically

stockiNail commented 2 years ago

Not sure what the reason was behind re exporting string as another type without actually doing something with it but I think this is better and more type safe that what it was before

@LeeLenaleee we have tried in the past that format but it seems is not compatible with some TS version... I'm looking for the comment in a PR

LeeLenaleee commented 2 years ago

Guess it is anything lower as 4.1 @stockiNail. Because that is when it was released: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html

But since it is a V2 release I guess we can call it a breaking change and require at least TS version 4.1 since 4.1 is already 2 years old

EDIT: Guess we also need to put a note in the migration guide then

stockiNail commented 2 years ago

Guess it is anything lower as 4.1 @stockiNail. Because that is when it was released: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html

But since it is a V2 release I guess we can call it a breaking change and require at least TS version 4.1 since 4.1 is already 2 years old

Could make sense for me. If all agree, I need to change this PR in order to

remove the tests on previous TS 4.1 version
add a note in the migration guide
~~add tag as breaking change~~

Let me know. As said, for me it's fine.