-
-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add typescript #53
feat: add typescript #53
Conversation
I did this to make the files more consistent
This reverts commit a48336d.
…corrections with real changes later
This should lay a foundation for future update, but they should be done incrementally
Updated jest, added babel typescript for jest, added a type to CartProvider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @getTobiasNielsen 👋🏻
Thanks for the PR 😻
I've made a few comments, and code suggestions, of which will require some further adjustments to make it work again (e.g. React.useEffect
etc.), and you'll need format with prettier again (good catch on the file name), as I removed some of the config so we only specify the overrides, and fallback to the defaults.
We could probably further tidy up the rollup config as the typescript plugin will probably cover some of the stuff I have going on in there.
If you make the changes, I'll see what @craigtweedy thinks, and we'll get this merged in.
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
Co-authored-by: Jamie Barton <jamie@notrab.dev>
…ConsistentCasingInFileNames": true,
With these last flags, TS is now fully strict, all that remains is to make the few "any" types more typed. |
@getTobiasNielsen looking good! I'll take a proper look over this over the next few days 😄 |
I think we're good for the handler as it will return early https://github.com/notrab/react-use-cart/pull/53/files#diff-0b5adbfe7b36e4ae2f479291e20152e33e940f7f265162d77f40f6bdb5da7405R195 |
Yes, but it's still being triggered by the addItem function, which will be confusing for people using it, maybe? I guess, I don't know what the handler is actually being used for in the real world, so hard to say. |
Not sure if this needs anything else. Let me know. |
🎉 This PR is included in version 1.10.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* feat: add typescript (#53) * Update prettierrc.config.js I did this to make the files more consistent * Revert "Update prettierrc.config.js" This reverts commit a48336d. * Update prettierrc.config.js * Renamed prettier config properly https://prettier.io/docs/en/configuration.html * Hit the most important files with prettier, to avoid mixing prettier corrections with real changes later * Added typescript, and also a very open tsconfig This should lay a foundation for future update, but they should be done incrementally * Added ts supprt and fixed a type declaration path mistake Updated jest, added babel typescript for jest, added a type to CartProvider * Update prettier.config.js Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update prettier.config.js Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update prettier.config.js Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update prettier.config.js Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update prettier.config.js Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update src/useLocalStorage.ts Co-authored-by: Jamie Barton <jamie@notrab.dev> * Update src/useLocalStorage.ts Co-authored-by: Jamie Barton <jamie@notrab.dev> * Added some of the updates * Added * as React * Added some interfaces, some strictness, some looseness * Fixed typo * Added optional on the CartProvider props * Missed an any, will need a proper type * Added payload, instead of item * Fixed typo * Turned on "noImplicitAny": true, "strictNullChecks": true, and "forceConsistentCasingInFileNames": true, * Changed the last flags to true. TS is fully strict now. * Stronger typed CartProvider props * Little stronger storage type Co-authored-by: Jamie Barton <jamie@notrab.dev> * chore: update release config (#54) (#55) * chore(pkg): update keywords Co-authored-by: getTobiasNielsen <54803528+getTobiasNielsen@users.noreply.github.com>
@getTobiasNielsen released! Let me know if you run into anything 👍🏻 Great work! 🥇 |
First, I fiddled with prettier, made it more strict, and also renamed it, I believe the name of the config file was wrong was wrong.
Then I hit most of the files, that we are going to be working on with these new prettier settings, just to get those changes out of the way, before the logic changes, to make it less muddy.
I then added typescript, and upgraded jest and added a standard babel plugin, to make jest eat typescript files.
The TS config is very loose, this is on purpose, as this will allow us to slowly convert everything bid by bid, and make the config more strict as it comes along.
I added minor actual changes, to make the code compile., mainly added a few
<any>
s, and made CartProvider a React.FC type