GriddleGriddle / Griddle

Simple Grid Component written in React
http://griddlegriddle.github.io/Griddle/
MIT License
2.5k stars 378 forks source link

Making typescript definitions comply with strict checking #727

Closed Errorific closed 7 years ago

Errorific commented 7 years ago

Griddle major version

1.8.0

Changes proposed in this pull request

Type GriddleActions in alignment with strict checking requirements and change typescript config to be strict by default.

Why these changes are made

Downstream projects should be able to use the library without having to comply to the libraries policies regarding the strictness of the typescript compiler. The definitions provided as of 1.8.0 don't pass checks for strictNullChecks as the optional keys in GriddleActions are all implicitly typed as 'X | undefined' but that isn't explicit in the objects type. If Griddle's typings are as strict as possible then it will compile in any downstream projects regardless of how lenient or strict they decide to set themselves up. Given it's a 1 line fix and no other strictness violations were found I've also turned on the other strict compiler options to catch this in the future.

Are there tests?

I've added a check-ts task to the package file that checks the file. There's probably a way to pack this into a js test file that I'm not aware of.

dahlbyk commented 7 years ago

Thanks for this, @Errorific! This is really closely related to https://github.com/GriddleGriddle/Griddle/pull/724, so I've merged part of it into that branch to test it out.

I have specifically avoided enabling the strict settings in tsconfig.json because I want to keep a low barrier to entry for contributing Storybook entries to stories/index.tsx. But it looks like check-ts doesn't actually depend on those settings. How does https://github.com/GriddleGriddle/Griddle/pull/724/commits/7ece2684da9979d8d243783399fd2b563b2c77b6 look to you?

Errorific commented 7 years ago

Looks fine, and the tsconfig bit makes sense with the storybook stuff being added.