From 70ef156f6d9b7db041eb925f96c6cf228b94769a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=ADcholas=20Andr=C3=A9?= Date: Fri, 31 May 2024 14:03:53 -0300 Subject: [PATCH] addressing feedback --- _includes/markdown/Typescript.md | 59 +++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/_includes/markdown/Typescript.md b/_includes/markdown/Typescript.md index 26253dd1..1aaa5ba7 100644 --- a/_includes/markdown/Typescript.md +++ b/_includes/markdown/Typescript.md @@ -6,33 +6,41 @@ We believe that: 1. TypeScript can make engineers more confident 2. TypeScript makes refactoring code easier 3. TypeScript helps engineers unfamiliar with the codebase +4. TypeScript helps catch certain issues earlier Therefore we are big fans of TypeScript at 10up and we strive to use it whenever it makes sense. +We also recgonize that the industry has largely adopted TypeScript as the defacto standard for writing JavaScript apps. + ## When to use TypeScript At 10up we require that all projects considered "JavaScript-First" to make use of TypeScript. Generally speaking Standard WordPress projects (including block development) would not fall under "JavaScript-First" projects. -If you are building a project using Next.js and/or 10up's [HeadstartWP framework](https://headstartwp.10up.com/), Sanity and any other headless CMS'es powered by a React front-end then we require TypeScript unless other circumstances make using just JavaScript a better choice. +If you are building a project using Next.js and/or 10up's [HeadstartWP framework](https://headstartwp.10up.com/), Sanity and any other headless CMS'es powered by a React front-end then we require TypeScript unless other circumstances make using JavaScript a better choice. -The main reason we don't require TypeScript for standard WordPress projects at the moment is that TypeScript support for block development is still very limited, largely driven by community types packages that don't always match the actual types for the `@wordpress` packages. +The main reason we don't require TypeScript for standard WordPress projects is that TypeScript support for block development is still very limited, largely driven by community types packages that don't always match the actual types for the `@wordpress` packages. ## How to use TypeScript -In an ideal world, every engineer would be fully capable of writing all sorts of complex TypeScript types, however, we understand that it takes time to master a language like TypeScript, especially in a few scenarios that require writing more complex types. Therefore, while we use a reasonably strict `tsconfig.json` we are flexible in practice. +In an ideal world, no one would struggle with TypeScript, however, we understand that it takes time to master a language like TypeScript, especially in a few scenarios that require writing more complex types. Therefore, while we use a reasonably strict `tsconfig.json` we are flexible in practice. + +We also don't want engineers looking for "perfection" when it comes to writing complex types. TypeScript has a very powerfull type system and it allows for amazing things to be done statically at compilation level, however, we should recognize that most projects will suffice with simpler, easy-to-use types. Unless there's a good reason (e.g building a reusable package or library), engineers should strive to not write overly complex types. -We want the TypeScript to always be valid and issue no errors at compilation, but we allow a few escape hatches should engineers get stuck with a particular typing problem. We follow the principles below when writing TypeScript: +Lastly, we want TypeScript to always be valid and issue no errors at compilation, but we allow a few escape hatches should engineers get stuck with a particular typing problem. We follow the principles below when writing TypeScript: ### 1. Never ship code that has compilation issues Depending on the tooling you're using (e.g. Next.js) it is possible to disable type checking and still compile invalid TS code as long as the underlying JS code is valid. While the runtime behavior might still be working as expected this is not a recommended practice and generally would just make engineers not have trust in the type system. If you are facing a challenge typing something it is better to use some of the escape hatches documented below instead of just ignoring TS errors. ### 2. Favor efficiency instead of strict typing + In an ideal world, all engineers would be capable of typing everything properly from the get-go. However, we know that is not the case and engineers need time and practice to get there. For that reason, we favor efficiency over strict typing. +By "efficiency" we mean engineers recognizing and time-boxing the time spent trying to type something properly and not letting that block the development of the task at hand. We prefer to ship non-strict typed code instead of sinking a lot of time on it. + Our philosophy is that over time engineers will organically get more familiar with TypeScript and strictness will come as a result. Code reviews are also a place where engineers might get quick feedback that would allow them to more efficiently address typing issues instead of wasting a lot of time during initial development trying to figure it out on their own. -When facing such typing challenges we recommend the following strategies as an escape hatch: +To favor efficiency, we recommend the following "escape hatches": #### 1. The `as unknown as ExpectedType` type cast @@ -55,12 +63,18 @@ Note: we recommend setting the `noImplicitAny` setting to `true` in `tsconfig.js Lastly, if nothing else worked to unblock the code, TypeScript allows disregarding errors on a given line by just telling the compiler that an error is expected on that line. This is usually the least effective option as it would only solve the problem for a single line. However, if nothing else works, this is the last and most drastic escape hatch. +When using any of these escape hatches, please leave a `TODO` comment with a short description of the issue: + +```js +// TODO(nicholaiso): the types coming from the package do not seem correct +``` + While we'd like none of these escape hatches to be used, it would be unrealistic to expect all engineers to have types 100% correct on every project all the time. We believe that these recommendations provide a path to address them organically during the life-cycle of the project. ## Tools - [TypeScript Error Translator](https://ts-error-translator.vercel.app/): Lets you drop any TypeScript error message and gives you a human-readable explanation in plain English. -- [pretty-ts-errors] (https://marketplace.visualstudio.com/items?itemName=yoavbls.pretty-ts-errors): A VSCode extension that translaters TypeScript errors in a human-readable way right into VSCode. +- [pretty-ts-errors](https://marketplace.visualstudio.com/items?itemName=yoavbls.pretty-ts-errors): A VSCode extension that translaters TypeScript errors in a human-readable way right into VSCode. ## TypeScript and CI @@ -85,21 +99,40 @@ We also have a recommended [tsconfig.json](https://github.com/10up/headstartwp/b While `interface` and `type` are mostly interchangeable, prefer `type` over `interfaces` unless you expect the props to be the base for other components's props. -```ts +```tsx type BlocksProps = { html: string; }; -export const Blocks = ({ html }: BlocksProps) => { +export const Blocks: React.FC = ({ html }) => { // render blocks } ``` +### Use `React.FC` to type React Components + +When typing React components prefer using `React.FC`. While in the past it was problematic because of implicitly accepting `children` as an optional prop, since React 18 it [no longer does that](https://www.totaltypescript.com/you-can-stop-hating-react-fc). Using React.FC ensures your component is correctly typed, including its return type. + +```tsx +import * as React from 'react'; + +type MyComponentProps = { + /* */ +}; + +export const MyComponent: React.FC = (props) => { + // 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. + +The [React TypeScript Cheatsheet](https://react-typescript-cheatsheet.netlify.app/) is a good resource to check how to type specific things in React (e.g Forms, Events etc). + ### Use function default arguments instead of `defaultProps` `defaultProps` has been deprecated in React 19, therefore prefer using function default arguments. -```ts +```tsx type MyComponentProps = { title?: string; }; @@ -115,12 +148,12 @@ Make sure the eslint rule `react/require-default-props` is set up to look for `d `PropsWithChildren` will make `children` optional, but sometimes you do want to be very explicit and make it required. Therefore we generally recommend declaring `children` explicitly. -```ts +```tsx type LayoutProps = { - children: React.Node + children: React.Node; }; -// Layout has passed a children prop +// If Layout is not passed a children, TS will catch the error const Layout = (props: LayoutProps) { return (
@@ -134,7 +167,7 @@ const Layout = (props: LayoutProps) { Prop spreading is when you simply forward all props to another component e.g: -```ts +```tsx // AVOID DOING THIS const PostProps = { title: string,