You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Describe the feature in detail (code, mocks, or screenshots encouraged)
As discussed in #2733, the next item I would like to tackle is refactoring the components to remove the return types.
This change has no performance change at all, but that doesn't mean there shouldn't be a change. There are currently 7 possible return types for a React component:
From React:
React.FC: This is what Skeleton currently uses. This defines a functional component
React.ReactElement: This is what JSX.Element gets mapped to. It's basically what it says on the tin, an element in a React DOM
React.ReactPortal: This is a ReactElement with a children property
React.ReactNode: This basically describes any node that can be inserted into a React DOM. This is what JSX.IntrinsicElement gets mapped to by default, unless overridden
React.JSX.Element: This is the replacement for the depreciated JSX global namespace
From TypeScript:
JSX.Element: This maps to React.ReactElement in a React project, but the JSX global namespace is depreciated
JSX.IntrinsicElement: This maps to React.ReactNode in a React project, but again the JSX global namespace is depreciated
As you can see, there are a lot of possible return types in React. The general consensus used to be to use React.FC, but React.FC has it's own fair share of problems. Then the consensus became to use JSX.Element, but that also had issues since it's kind of a black box as it's a generic ReactElement with no type parameters passed to it. So, now we're left in a strange spot, you can either type everything yourself manually (i.e. attempting to predict the outputted types from React.createElement) or just letting TypeScript infer the type. The general consensus amongst the React community has been to just allow TypeScript to infer the type.
I know this is a lot to take in. Please feel free to look over the links provided below. No change will be made to the code until there has been at least a week or so to allow for comment. The main person who should probably be involved in this one for now is @Hugos68 since he has done most of the React work to this point, and Chris is taking some much needed time off; however, if there is any feedback from the community that is appreciated as well.
Describe the feature in detail (code, mocks, or screenshots encouraged)
As discussed in #2733, the next item I would like to tackle is refactoring the components to remove the return types.
This change has no performance change at all, but that doesn't mean there shouldn't be a change. There are currently 7 possible return types for a React component:
From React:
React.FC
: This is what Skeleton currently uses. This defines a functional componentReact.ReactElement
: This is whatJSX.Element
gets mapped to. It's basically what it says on the tin, an element in a React DOMReact.ReactPortal
: This is aReactElement
with a children propertyReact.ReactNode
: This basically describes any node that can be inserted into a React DOM. This is whatJSX.IntrinsicElement
gets mapped to by default, unless overriddenReact.JSX.Element
: This is the replacement for the depreciatedJSX
global namespaceFrom TypeScript:
JSX.Element
: This maps toReact.ReactElement
in a React project, but theJSX
global namespace is depreciatedJSX.IntrinsicElement
: This maps toReact.ReactNode
in a React project, but again theJSX
global namespace is depreciatedAs you can see, there are a lot of possible return types in React. The general consensus used to be to use
React.FC
, butReact.FC
has it's own fair share of problems. Then the consensus became to useJSX.Element
, but that also had issues since it's kind of a black box as it's a genericReactElement
with no type parameters passed to it. So, now we're left in a strange spot, you can either type everything yourself manually (i.e. attempting to predict the outputted types fromReact.createElement
) or just letting TypeScript infer the type. The general consensus amongst the React community has been to just allow TypeScript to infer the type.I know this is a lot to take in. Please feel free to look over the links provided below. No change will be made to the code until there has been at least a week or so to allow for comment. The main person who should probably be involved in this one for now is @Hugos68 since he has done most of the React work to this point, and Chris is taking some much needed time off; however, if there is any feedback from the community that is appreciated as well.
What type of pull request would this be?
Other
Provide relevant links or additional information.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/266eae5148c535e6b41fe5d0adb2ad23f302bc8a/types/react/index.d.ts#L3182
facebook/create-react-app#8177
typescript-cheatsheets/react#643
https://stackoverflow.com/questions/76224848/what-is-the-correct-return-type-replacement-for-jsx-element-after-the-global-jsx
https://stackoverflow.com/questions/58123398/when-to-use-jsx-element-vs-reactnode-vs-reactelement
The text was updated successfully, but these errors were encountered: