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

React 18: Warn when components render undefined #3020

Open
nickmccurdy opened this issue Jul 16, 2021 · 8 comments · May be fixed by #3750
Open

React 18: Warn when components render undefined #3020

nickmccurdy opened this issue Jul 16, 2021 · 8 comments · May be fixed by #3750

Comments

@nickmccurdy
Copy link

nickmccurdy commented Jul 16, 2021

Upstream: facebook/react#21869

In React 18 (currently in alpha), components may render undefined, and React will render nothing to the DOM instead of throwing an error. This allows components that don't render anything useful (such as optional rendering or components that rely on side effects from hooks or lifecycles) to return undefined instead of explicitly returning null. However, accidentally rendering nothing in a component could still cause surprises (which is why require-return-return exists, though it's only for class components). Additionally, a team may want to have a code style guideline of explicitly returning undefined to prevent components from accidentally doing nothing. The React team seems to be recommending that users use linters to catch this issue in React 18 (since React itself will no longer warn), though it's up to debate whether this is something we'd want to enforce by default, as not rendering anything is valid as of React 18.

Possible approaches

Add functionality to require-render-return

We could extend this rule to include function components that return undefined and class components that explicitly return undefined. Documentation should be updated to reflect that this rule affects more components and may not be necessary in React 18. This would technically be a breaking change for React 18 users, as it's potentially valid for them to return undefined, and the rule is recommended.

Add new rule

Rather than put all functionality in an existing rule that could affect existing configs, we could create a separate rule such as no-render-return that covers all these use cases. We may want to deprecate and stop recommending require-render-return in this case, as it would have a subset of this rule's functionality.

@golopot
Copy link
Contributor

golopot commented Jul 18, 2021

Can you provide examples that should be warned?

@nickmccurdy
Copy link
Author

nickmccurdy commented Jul 18, 2021

const Hello = () => {
};

const Hello = () => {
  <div>Hello</div>;
};

const Hello = () => {
  return undefined;
};

// Overlaps with require-render-return
const Hello = createReactClass({
  render() {
  },
});

// Overlaps with require-render-return
const Hello = createReactClass({
  render() {
    <div>Hello</div>;
  },
});

const Hello = createReactClass({
  render() {
    return undefined;
  },
});

// Overlaps with require-render-return
class Hello extends React.Component {
  render() {
  }
}

// Overlaps with require-render-return
class Hello extends React.Component {
  render() {
    <div>Hello</div>;
  }
}

class Hello extends React.Component {
  render() {
    return undefined;
  }
}

@ljharb
Copy link
Member

ljharb commented Jul 19, 2021

It seems very unfortunate and quite hostile to pass the buck for this check to the linting ecosystem, but of course we’ll add a way to check for this if react leaves us no choice.

@nickmccurdy
Copy link
Author

nickmccurdy commented Jul 19, 2021

To be clear I'm not involved in the decision or changes made in the React core. I just opened this issue because it doesn't seem like there's an official lint rule for this change yet.

That being said, while this issue is primarily for React 18 support, some React 17 users may find it useful to have a lint warning in their editor so they don't have to check for the runtime error.

@nickmccurdy
Copy link
Author

nickmccurdy commented Jul 19, 2021

Do we care about warning when there's an explicit undefined return (return undefined)? If so, this would probably make more sense as a new rule. If not, we could treat it as an intentional override from the user and ignore it, and add the new function component warnings to require-render-return (breaking change, unless we temporarily disable function component checking via config and update the default later).

Edit: I've crossposted the leading question in case the React team has feedback on it.

@ljharb
Copy link
Member

ljharb commented Jul 21, 2021

Yes, ideally whenever we can statically detect an undefined is being returned, users should be warned.

This includes return;, return undefined; var x; return x; etc.

@akulsr0
Copy link
Contributor

akulsr0 commented May 7, 2024

@ljharb Is this still open to pickup?

Also for functional components, how do we differentiate between whether it's a component or normal function? Assuming this needs to support FC too

@ljharb
Copy link
Member

ljharb commented May 8, 2024

@akulsr0 yep!

We have existing utilities for that, but they may need updating - for example, a component that does something like useEffect(fn); return undefined; should be warned because it's using hooks (or jsx, etc) but a function returning undefined that doesn't seem like a component shouldn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants