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

BASIC - Rework Function Components section to prefer normal function … #104

Merged
merged 4 commits into from
Apr 15, 2019
Merged

Conversation

Retsam
Copy link
Contributor

@Retsam Retsam commented Apr 13, 2019

This is a change, related to some of the discussion in #90 and #87 about React.FC/React.FunctionComponent. I realize this change itself is pretty opinionated, as is some of my wording, (though I did attempt to be fair to both sides), but I thought it'd be a good starting point for discussion.


There's two syntaxes for declaring function components:

// 'normal function' syntax
const C1 = ({message}: {message: string}) => <div>{message}</div>;

const C2: React.FC<{message: string}> => <div>{message}</div>;

As currently written, the can either be read as neutral or favoring the React.FC syntax: the main section is written neutrally (you can use X or Y) but the sub-points are largely written favoring the FC syntax.

This this restructures the section so that the "normal function" syntax is preferred, without strictly recommending against the React.FC syntax.

Most of my arguments for this change are in the new version, but I'll reiterate them here:

  • React.FC doesn't handle defaultProps correctly:
const Component: React.FC<message: string> = ({message}) => <div>{message}</div>
Component.defaultProps = {message: "Hello, there"};
const test = <Component />; // Error, message is required.
  • There may be some issues with the type of the implicit children prop: I listed [React] React.FC doesn't allow bare return of children DefinitelyTyped/DefinitelyTyped#33006 in relation to this point, though perhaps the root cause there is something deeper: that issue ends up linking to some deeper type issues, so maybe those would eventually be resolved. (So maybe I need to remove that link from my PR)

  • Personally, I think explicit children is better than implicit children, anyway. The props are the contract for a component and if a component expects to have children, that's an important part of the contract. Also, children can be mandatory with an explicit children prop:

// Errors if used like `<MustHaveChildren />`
const MustHaveChildren = ({children}: {children: React.ReactNode}) => <div>{children}</div>;
  • Ultimately, there's just not much benefit of React.FC to justify the extra characters: I guess it prevents errors like MyComponent.propType = 'MyComponent' (should be propTypes), and an error like const MyComponent = () => ({not: 'a valid component'}).

  • Plus, all else being equal, I think it's good for a cheatsheet to have a single clear recommendation, rather than presenting two equivalent options as equal.


As a separate commit, I also moved the props into their own type. Personally I find that's a better template as it quickly becomes unreadable to inline more than a couple props. (I can split that into its own PR or drop it)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
ferdaber and others added 2 commits April 15, 2019 11:37
Co-Authored-By: Retsam <Retsam@users.noreply.github.com>
Co-Authored-By: Retsam <Retsam@users.noreply.github.com>
@ferdaber ferdaber merged commit 0794f1d into typescript-cheatsheets:master Apr 15, 2019
@swyxio
Copy link
Collaborator

swyxio commented Apr 17, 2019

sweet

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

Successfully merging this pull request may close these issues.

3 participants