-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Comments
Can you provide examples that should be warned? |
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;
}
} |
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. |
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. |
Do we care about warning when there's an explicit undefined return ( Edit: I've crossposted the leading question in case the React team has feedback on it. |
Yes, ideally whenever we can statically detect an This includes |
@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 |
@akulsr0 yep! We have existing utilities for that, but they may need updating - for example, a component that does something like |
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 returnundefined
instead of explicitly returningnull
. However, accidentally rendering nothing in a component could still cause surprises (which is whyrequire-return-return
exists, though it's only for class components). Additionally, a team may want to have a code style guideline of explicitly returningundefined
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 returnundefined
. 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 returnundefined
, 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 recommendingrequire-render-return
in this case, as it would have a subset of this rule's functionality.The text was updated successfully, but these errors were encountered: