-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add type checking #285
Comments
nothing against that.... actually I'm in favour of any tools that in some how help improving the code quality. (even more with Dynamic languages ) |
Definitely in favor of type checking, I think typescript is "winning" (https://insights.stackoverflow.com/trends?tags=typescript%2Cflowtype) but either would be fine. Is there a way that we can incrementally roll it out? |
I'm definitely in favour of Typescript... the only reason why we didn't use it if that it was too much learning for Jrs... Grads that need to learn JS + React really struggle to learn also Typescript |
Looks like it should be possible to gradually roll it out https://github.com/microsoft/TypeScript-React-Conversion-Guide, I've had a bit of a play this morning and it seems fairly simple, the only thing I'm not sure on is the bundling of the app when we build it? I guess types are a little confusing at first, but they should also help to guard against some of the issues more junior developers come across e.g. not passing a prop that a component needs. With typescript they'd get these warnings displayed before the app has even compiled again Personally when I was working on updating the categorisation page some types would have been super helpful so that I could make sure that the update I made matched the old schema |
It also is basically a unit test for everything as the compiler just
catches simple errors which otherwise you’d need to explicitly test for. So
I’m definitely on board. So let’s get it set up, target the categorization
stuff first, and then just put in some backlog items to convert everything
else?
…On Sat, Mar 14, 2020 at 09:09 Tom Munro ***@***.***> wrote:
Looks like it should be possible to gradually roll it out
https://github.com/microsoft/TypeScript-React-Conversion-Guide, I've had
a bit of a play this morning and it seems fairly simple, the only thing I'm
not sure on is the bundling of the app when we build it?
I guess types are a little confusing at first, but they should also help
to guard against some of the issues more junior developers come across e.g.
not passing a prop that a component needs. With typescript they'd get these
warnings displayed before the app has even compiled again
Personally when I was working on updating the categorisation page some
types would have been super helpful so that I could make sure that the
update I made matched the old schema
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#285 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH7EJMAHR5A6KUAZI2JK5LRHNCTJANCNFSM4LG4F22Q>
.
|
Sounds good to me, I'm just about to raise a pr with some basic changes |
At work we use flow to help enforce our type checking in the app and it makes development significantly faster as well as helping people unfamiliar to a part of the project as they get warnings when types/ imports are wrong
What are peoples thoughts on rolling out something more across the app?
@neilfulwiler @sebastianovide
The text was updated successfully, but these errors were encountered: