Skip to content
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

awesome advice from Ferdaber #57

Closed
ferdaber opened this issue Dec 14, 2018 · 13 comments
Closed

awesome advice from Ferdaber #57

ferdaber opened this issue Dec 14, 2018 · 13 comments

Comments

@ferdaber
Copy link
Collaborator

Hi! I'm one of the contributors/maintainers to the @react/types library in DT, and I just have some suggested changes to the docs to ensure that folks who start with TS in React has a smoother experience!

For the section on Function Components:

  • A common pitfall is that these patterns are not supported:
const MyConditionalComponent = ({ shouldRender = false }) => shouldRender ? <div /> : false
const MyArrayComponent = () => Array(5).fill(<div />)
const el = <MyConditionalComponent /> // throws an error
const el2 = <MyArrayComponentt /> // throws an error

This is because due to limitations in the compiler, function components cannot return anything other than a JSX expression or null, otherwise it complains with a cryptic error message saying that the other type is not assignable to Element. Unfortunately just annotating the function type will not help so if you really need to return other exotic types that React supports, you'd need to perform a type assertion:

const MyArrayComponent = () => Array(5).fill(<div />) as any as JSX.Element

For the section on Class Components:

  • I recommend annotating the state class property in addition to adding it as the 2nd generic type parameter in the base class, because it allows better type inference when accessing this.state and also initializing the state. This is because they work in two different ways, the 2nd generic type parameter will allow this.setState() to work correctly, because that method comes from the base class, but initializing state inside the component overrides the base implementation so you have to make sure that you tell the compiler that you're not actually doing anything different.
type MyState = {}
class App extends React.Component<{}, MyState> { state: MyState = {} }

For the section on Typing DefaultProps:

  • I strongly do not recommend annotating defaultProps into a Partial of your Props interface. This causes issues that are a bit complex to explain here with the type inference working with JSX.LibraryManagedAttributes. Basically it causes the compiler to think that when creating a JSX expression with that component, that all of its props are optional. Don't do this! Instead this pattern is recommended:
type Props = Required<typeof MyComponent.defaultProps> & { /* additional props here */ }

export class MyComponent extends React.Component<Props> {
  static defaultProps = {
    foo: 'foo'
  }
}

For the section on Forms and Events:

  • I would just add that inlining the event handler when you don't need to optimize is recommended for better type inference:
// instead of this:
const myHandler = (event: React.MouseEvent<HTMLButtonElement>) => {}
const el = <button onClick={myHandler} />

// do this:
const el = <button onClick={event => {}} />

This tells the compiler that there is no ambiguity to when the handler is being used, and adding function types to the call site allows it to infer the event parameter's types right away.

That's all for now. I can definitely make a PR if y'all agree to these changes!

@swyxio
Copy link
Collaborator

swyxio commented Dec 14, 2018

hey Ferdy! wow, thank you for maintaining @types/react and giving feedback! no PR needed, i'll incorporate your advice myself.

may i ask - how did you get started contributing to @types/react and are there any resources you recommend reading on it? (especially historical/meta articles on how @types/react has evolved, is managed, and lessons we have learned) I feel like its basically undocumented because i couldnt find anything good on it - and if theres one thing i can do well i can help you write documentation. do you need more contributors?

swyxio added a commit that referenced this issue Dec 14, 2018
@ferdaber
Copy link
Collaborator Author

I started when TS 3.0 was in the works and LibraryManagedAttributes were added as a feature (I implemented it in the React types with a lot of help from existing contributors and folks from the TS team).

One of the contributors is working on creating a large breaking change (potentially) to make the types much stronger, but at the cost of potentially breaking a lot of existing code, so she is working through the kinks on that.

As far as historical info goes, there's not much honestly, a lot of "this happened or was implemented because X" is scattered among closed issues in the DT repository. It would be nice to document why things are the way they are, but it may not be worth it if a lot of it is going away soon.

That said, I really like this repository as a place to build off of because there's not much out there that documents best practices with using Typescript and JSX (and with React, specifically). I really like this section of the TS handbook as a base to get really familiar with how the compiler works with JSX: https://www.typescriptlang.org/docs/handbook/jsx.html, a lot of why things are done they way they are in the React types spins off of these behaviors.

@swyxio
Copy link
Collaborator

swyxio commented Dec 15, 2018

ok cool. yeah i'll try to keep this a tightly scoped best practices doc.

what issue is this large breaking change? would love to check it out.

@ferdaber
Copy link
Collaborator Author

microsoft/TypeScript#28954
which is dependent on some better resolution on microsoft/TypeScript#14729 (or at least being able to appropriately inject a generic type parameter into JSX.Element)

@azizhk
Copy link
Collaborator

azizhk commented Dec 27, 2018

Hi @sw-yx @ferdaber ,
I had a doubt regarding on of the pitfalls that you mention.
You mention to use as JSX.Element while currently the Readme mentions React.ReactNode as the best option.
Are there any differences between the two? Should we use JSX.Element where we are targetting interoperability between react and preact? Or JSX.Element is the generic term for all VDOM libraries and React/Preact Developers should use React.ReactNode only.

@azizhk
Copy link
Collaborator

azizhk commented Dec 27, 2018

Ok, I realize JSX.Element does not contain string. So I guess React.ReactNode is the better option.

@swyxio
Copy link
Collaborator

swyxio commented Dec 28, 2018

good :)

@ferdaber
Copy link
Collaborator Author

ferdaber commented Jan 4, 2019

A more technical explanation is that a valid React node is not the same thing as what is returned by React.createElement. Regardless of what a component ends up rendering, React.createElement always returns an object, which is the JSX.Element interface, but React.ReactNode is the set of all possible return values of a component.

  • JSX.Element -> Return value of React.createElement
  • React.ReactNode -> Return value of a component

@swyxio swyxio changed the title Suggested changes awesome advice from Ferdaber Jan 5, 2019
@swyxio
Copy link
Collaborator

swyxio commented Jan 5, 2019

putting that in

@ferdaber
Copy link
Collaborator Author

ferdaber commented Jan 11, 2019

@sw-yx there was a weird grammatical error in my post above which is quoted, which I have corrected. Can you fix inside the README? 😅

@swyxio
Copy link
Collaborator

swyxio commented Jan 11, 2019

done. im also just gonna invite you to help maintain this repo if you want :)

@jperasmus
Copy link

Another alternative to using the type casting for this: const MyArrayComponent = () => Array(5).fill(<div />) as any as JSX.Element, is to use a fragment.

const MyArrayComponent = () => <>{Array(5).fill(<div />)}</>

@mikew
Copy link

mikew commented Mar 17, 2021

Yeah kind of shocked to see anything that advocates for typescript also recommending throwing in as any as ....

bernssolg added a commit to bernssolg/My-React-Sample that referenced this issue Feb 28, 2022
erinodev added a commit to erinodev/My-React-project that referenced this issue Feb 28, 2022
petardev101 added a commit to petardev101/react that referenced this issue Jun 4, 2022
supercrytoking added a commit to supercrytoking/react that referenced this issue Jul 14, 2022
kevindavies8 added a commit to kevindavies8/react-full-stack-developer that referenced this issue Aug 24, 2022
johnfrench3 pushed a commit to johnfrench3/react-Fronted-developer that referenced this issue Sep 7, 2022
ericbrown2716 added a commit to ericbrown2716/react-stack-build-website that referenced this issue Sep 29, 2022
peterjohnson4987 added a commit to peterjohnson4987/full-stack-developer-react that referenced this issue Oct 3, 2022
renawolford6 pushed a commit to renawolford6/react-husky-website that referenced this issue Oct 6, 2022
Yoshidayoshi23 added a commit to Yoshidayoshi23/react that referenced this issue Oct 20, 2022
renawolford6 added a commit to renawolford6/react-dev-build-doc- that referenced this issue Nov 10, 2022
coopfeathy added a commit to coopfeathy/cheatsheet that referenced this issue Dec 4, 2022
dreamcoder75 added a commit to dreamcoder75/react-sample that referenced this issue Jan 15, 2023
holyblock pushed a commit to holyblock/chart that referenced this issue Feb 27, 2023
AIDevMonster added a commit to AIDevMonster/Awesome-React that referenced this issue Jun 21, 2023
whiteghostDev added a commit to whiteghostDev/Awesome-React that referenced this issue Aug 6, 2023
cedev935 added a commit to cedev935/React-TypeScript that referenced this issue Sep 11, 2023
aleksandaralek added a commit to aleksandaralek/typescript-react-cheatsheet that referenced this issue Oct 24, 2023
xbucks pushed a commit to xbucks/react-cheatsheets that referenced this issue Oct 24, 2023
joyfulmagician added a commit to joyfulmagician/react that referenced this issue Oct 25, 2023
KonohaBrain125 pushed a commit to KonohaBrain125/React-Typescript that referenced this issue Oct 26, 2023
TOP-10-DEV added a commit to TOP-10-DEV/typescript-cheatsheets-react that referenced this issue Dec 8, 2023
secretsuperstar1109 added a commit to secretsuperstar1109/react-typescript-cheatsheets that referenced this issue Dec 9, 2023
champion119 added a commit to champion119/react that referenced this issue Jan 5, 2024
dragon360-dev added a commit to dragon360-dev/react that referenced this issue Mar 13, 2024
EugeneYoona added a commit to EugeneYoona/React_full_src that referenced this issue Apr 10, 2024
fairskyDev0201 added a commit to fairskyDev0201/typescript-cheatsheet that referenced this issue Apr 17, 2024
TechSolutionNinja added a commit to TechSolutionNinja/react that referenced this issue May 27, 2024
alisenola added a commit to alisenola/react-cheatsheets that referenced this issue May 29, 2024
LegendaryDev320 added a commit to LegendaryDev320/react that referenced this issue Jun 5, 2024
kaleb0402 added a commit to kaleb0402/react that referenced this issue Jul 18, 2024
Linda423 added a commit to Linda423/React that referenced this issue Jul 31, 2024
tosky19941209 added a commit to tosky19941209/React that referenced this issue Aug 7, 2024
touchsky941209 added a commit to touchsky941209/react-rebrand that referenced this issue Aug 26, 2024
genie4viz pushed a commit to genie4viz/react that referenced this issue Sep 2, 2024
hussammousa68 added a commit to hussammousa68/react that referenced this issue Oct 13, 2024
zeus-soft-world added a commit to zeus-soft-world/React-TypeScript that referenced this issue Oct 15, 2024
chivalrousdev added a commit to chivalrousdev/React that referenced this issue Nov 14, 2024
ElegantDev43 added a commit to ElegantDev43/react-test that referenced this issue Dec 15, 2024
firedev200043 added a commit to firedev200043/react-test that referenced this issue Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants