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

fix(react): ✨ Update react templates #4679

Closed
wants to merge 1 commit into from

Conversation

virus2016
Copy link

Prefix interfaces with I. Have an arrow function as default. Lookup correct type if user is using React.FC as props.

Current Behavior

The React template creates the below:

import React from 'react';

import './header.module.scss';

/* eslint-disable-next-line */
export interface HeaderProps {}

export function Header(props: HeaderProps) {
  return (
    <div>
      <h1>Welcome to header!</h1>
    </div>
  );
}

export default Header;

Expected Behavior

import React from 'react';

import './header.module.scss';

/* eslint-disable-next-line */
export interface HeaderProps {}

export const Header: React.FC<HeaderProps> = (props) {
  return (
    <div>
      <h1>Welcome to header!</h1>
    </div>
  );
}

export default Header;

Related Issue(s)

#4677

Prefix interfaces with I. Have an arrow function as default. Lookup correct type if user is using React.FC as props.
@bcheidemann
Copy link
Contributor

Hey guys, I've looked through the code and it looks fine. Would really love for this to be merged because I really need it! 🙏

@puku0x
Copy link
Contributor

puku0x commented Feb 4, 2021

It seems React will have no default import in the future.
facebook/react#18102

I think this would be better.

import * as React from 'react';

@virus2016
Copy link
Author

It seems React will have no default import in the future.
facebook/react#18102

I think this would be better.

import * as React from 'react';

Thanks for this. I'll update this soon

@jaysoo
Copy link
Member

jaysoo commented Mar 16, 2021

My concern with using React.FC is that is isn't needed, and makes the function more awkwardly typed since it isn't a plain function anymore.

Some of the issues are outlined here: https://fettblog.eu/typescript-react-why-i-dont-use-react-fc/

And you could explicitly allow children with { children: React.ReactNode }, which I think is more clear that children is required or not. You can also use { children?: React.ReactNode} for optional children, and { children?: never }` if children isn't supported.

@jaysoo
Copy link
Member

jaysoo commented Mar 31, 2021

Since there hasn't been any feedback, I'm closing this PR. Feel free to continue discussion by opening an issue.

Like I said, I feel like React.FC is usually the wrong thing to reach for -- you're free to disagree with me :)

@jaysoo jaysoo closed this Mar 31, 2021
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants