web-dev-path icon indicating copy to clipboard operation
web-dev-path copied to clipboard

Add TypeScript

Open vmcodes opened this issue 2 years ago • 9 comments

What do we need to build or fix? We'd like to transition the site to TypeScript. This will likely include creating global types to be used, as well as creating standards for how to define props passed between components.

Technical details

  • Create a new branch named chore/add-typescript and checkout to it.
  • Create a types folder to keep global types and interfaces easily accessible.
  • Define standards for passing in props and where types should be used in general.
  • Determine if we should define functional components using the FC import from React, or if we should keep the current components as is and define the types inline.
  • Test the feature in different screen sizes and browsers to ensure it looks and functions as expected.
  • Commit the changes to the chore/add-typescript branch and create a pull request to merge the changes into the main branch.
  • Update the CHANGELOG.md file.

Approach suggestions There are a variety of valid approaches. We should probably discuss here in a thread what would be the best fit for Web Dev Path. Any thoughts or opinions should be included below, and would be appreciated.

Deadline Please remember that once you assign this task to yourself, you'll need to complete it in 3 weeks.

Acceptance criteria

  • Test the section and components in many screen sizes, you can use the Inspect tool for that.
  • Please test if the new changes added to the components do not affect the other instances.
  • Test the feature in many browsers, such as Chrome, Firefox, Edge, and Safari (MAC).
  • If there are any build problems when submitting your PR, run yarn build locally to solve the issues and commit the changes.
  • Update the CHANGELOG.md file.

vmcodes avatar Jun 12 '23 21:06 vmcodes

Thanks for creating this issue, @vmcodes !

While we are in the process of implementing a theme on issue #165 (or even before we start it), let's focus on this issue which aims to convert the project's js files to TypeScript (ts). This conversion will involve creating interfaces and global types for each component, making the project more resilient (as mentioned in this article: https://blog.bitsrc.io/5-strong-reasons-to-use-typescript-with-react-bc987da5d907) and better equipped for debugging and future testing.

In addition to determining who will work on this issue, it's crucial to decide on the approach. Should we assign it to a single developer, as we did with the styled-components implementation #162, or should we break it down into smaller issues to convert the project modularly? I'd appreciate hearing your thoughts on this matter @Web-Dev-Path/development-team

mariana-caldas avatar Jun 13 '23 01:06 mariana-caldas

In my opinion, we can break it down as there's not much variations in styles for typescript, unlike styled components

cherylli avatar Jun 13 '23 02:06 cherylli

I'm open to taking on the task alone, but if there's someone who wants to gain some TypeScript experience, I don't mind working with anyone.

As far as approach, I think that we shouldn't modify the component structure using something like:

const MyFunction : FC<{MyProps}> = ({ props }) =>

I think it'll just add confusion to an already solid foundation. We could do something more along the lines of:

export default function MyFunction({ props } : MyProps)

A question I had was, how strict do we want to be with types? Should we define all variables and avoid leaving anything undefined or as type any unless completely necessary?

Last question from me would be, do we want to keep all types in one file or define prop types within the component file if they aren't reused?

vmcodes avatar Jun 13 '23 03:06 vmcodes

even just const Component = ({prop}: PropsType) => {...} this is what i've been using

We should be strict otherwise there's no point using typescript. Only place we should use any is when we don't know what type we are getting e.g. like a generic API response but I don't think we will have that issue

cherylli avatar Jun 13 '23 08:06 cherylli

Hey, team! I've just converted to TypeScript this small React project and I think we can use this approach as a reference: https://github.com/mariana-caldas/react-interview-prep-playground

If we decide to tackle it with just one dev, I'd recommend starting converting the components first (not the pages).

mariana-caldas avatar Jun 14 '23 04:06 mariana-caldas

Hey, team! I've just converted to TypeScript this small React project and I think we can use this approach as a reference: https://github.com/mariana-caldas/react-interview-prep-playground

If we decide to tackle it with just one dev, I'd recommend starting converting the components first (not the pages).

do we have to use React.FC like in your test project? Actually never used it and I did some research I think we should stay away from it

https://github.com/facebook/create-react-app/pull/8177 https://medium.com/@harrymt/should-you-use-react-fc-for-typing-react-components-62cde9ba67c#

cherylli avatar Jun 14 '23 12:06 cherylli

Hey, team! I've just converted to TypeScript this small React project and I think we can use this approach as a reference: https://github.com/mariana-caldas/react-interview-prep-playground If we decide to tackle it with just one dev, I'd recommend starting converting the components first (not the pages).

do we have to use React.FC like in your test project? Actually never used it and I did some research I think we should stay away from it

facebook/create-react-app#8177 https://medium.com/@harrymt/should-you-use-react-fc-for-typing-react-components-62cde9ba67c#

Hey @cherylli and @vmcodes ,

By reading the article, it seems it is a matter of preference, however, because we're converting the project into TS (not creating it from scratch) it may be useful to stick with the traditional approach since it may help us to not miss any type annotation. So, I'd say, let's use the more beginner-friendly approach as per the reference.

mariana-caldas avatar Jun 14 '23 16:06 mariana-caldas

@mariana-caldas I'm cool with that. Were you able to view the draft PR I made? I can resolve the build issues if you think it would be good to at least change the file name extensions for now. You can write JS in TypeScript files and I thought it might be a good idea incase someone wants to start working on the theming issue at the same time. It would avoid a lot of merge conflicts.

vmcodes avatar Jun 14 '23 16:06 vmcodes

I wanted to share this insightful post by @cherylli on our Slack channel, where she explains why we should move away from using the React.FC approach in our TypeScript conversion.

image

In this React repository pull request, you can find a detailed explanation about why Facebook decided to drop React.FC from their projects. The primary reason is that React.FC introduces some limitations with TypeScript, which includes the following points:

  1. Implicit Children: React.FC automatically injects children into your component props, even if you don't want or need them. This can be problematic when you're trying to strictly define the props your component should accept, and children isn't one of them.

  2. Non-Standard Function Signatures: When using React.FC, the function signature becomes non-standard. A regular function allows for a variety of parameter types, whereas React.FC specifically requires an object.

  3. Generics Are More Difficult: React.FC can make it harder to correctly type generic props.

  4. Defaults and Required Flags: React.FC doesn't play nicely with defaultProps and the required flag in the context of TypeScript.

Considering these points, we can type our functional components like this:

type MyComponentProps = {
  name: string;
  age: number;
};

const MyComponent = ({ name, age }: MyComponentProps) => {
  // ...
}

Over the next few days, I'll be updating the project references to align with this approach. Thanks so much for highlighting this issue, Cheryl!

Please let me know your thoughts on this, team!

mariana-caldas avatar Jun 16 '23 00:06 mariana-caldas