React 18: Warn when components render `undefined`

[nickserv]

Upstream: https://github.com/facebook/react/pull/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]

Can you provide examples that should be warned?

[nickserv]

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]

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.

[nickserv]

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.

[nickserv]

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]

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]

@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]

@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.