Introducing TypeScript best practices #403
Conversation
|
|
||
| Note: we recommend setting the `noImplicitAny` setting to `true` in `tsconfig.json` which forces explicitly setting a type as any. This makes it easier to identify code that is relying on any and would also make catching the escape hatch in code reviews which is a great opportunity to get feedback and potentially get the types right. | ||
|
|
||
| #### 3. The `// @ts-expect-error` comment |
There was a problem hiding this comment.
Ideally this would be avoided if possible - casting as any should nearly always fix this issue. If you're having to use this - it may be a case where the engineer could ask for someone more experienced with TS to assist with the issue. Would also work as a great learning experience.
There was a problem hiding this comment.
In my experience the easiest way to assist with TS types for other engineers is once it's in a PR and I can see all the code. I've found Slack messages in isolation without seeing all the code hard to answer. So perhaps encourage them to get their work done with any and then TS types support comes once the PR is open. It also leaves a feedback trail for any other engineers on the project
There was a problem hiding this comment.
@kasparszarinovs I agree but I also feel like we need this as a last resort. Like @tobeycodes said this could potentially be a easy fix during code review.
Co-authored-by: Kaspars Zarinovs <[email protected]>
There was a problem hiding this comment.
@nicholasio great stuff! I just stumbled upon this and thought I'd give it a read! Found a few minor typos and left a comment for your consideration.
Great work!
| // component logic | ||
| } | ||
| ``` | ||
| **Note**: If using React < 17, be aware that `children` will be explicitly set for all components using `React.FC`. This can cause some TS errors when upgrading React to 18+, if a component is passed `children` but does not explicitly declare it as a prop. |
There was a problem hiding this comment.
While this is true, I'd say it's been a while since React 18 is out (2 years!) While it doesn't harm I don't feel this is as relevant now? Just flagging to avoid this feeling dated when it's born but feel free to disregard
There was a problem hiding this comment.
The only reason I added this was bc there might be projects still on React 17.
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
Co-authored-by: Antonio Laguna <[email protected]>
There was a problem hiding this comment.
Thanks @nicholasio! I found a few more easy fixes with suggestions.
Co-authored-by: Evan Mattson <[email protected]>
Co-authored-by: Evan Mattson <[email protected]>
| }; | ||
|
|
||
| // If Layout is not passed a children, TS will catch the error | ||
| const Layout = (props: LayoutProps) => { |
There was a problem hiding this comment.
@nicholasio should this be rewritten using React.FC as recommended above?
There was a problem hiding this comment.
Made a quick follow-up commit to update this thanks
This PR introduces a new TypeScript section and its best practices.