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.

Sign up for GitHub

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

Jump to bottom

test: Enable react/no-unstable-nested-components #34518

Merged
merged 1 commit into from Oct 17, 2022
Merged

test: Enable react/no-unstable-nested-components #34518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Expand Up @@ -159,6 +159,8 @@ module.exports = {
'react/state-in-constructor': 'off',
// stylistic opinion. For conditional assignment we want it outside, otherwise as static
'react/static-property-placement': 'off',
// Currently not in recommended ruleset but catches real bugs.
'react/no-unstable-nested-components': 'error',
},
overrides: [
{
Expand Down
14 changes: 5 additions & 9 deletions docs/data/material/guides/routing/ListRouter.js
View file
Open in desktop
Expand Up @@ -36,20 +36,16 @@ Router.propTypes = {
children: PropTypes.node,
};

const Link = React.forwardRef(function Link(itemProps, ref) {
return <RouterLink ref={ref} {...itemProps} role={undefined} />;
});

function ListItemLink(props) {
const { icon, primary, to } = props;

const renderLink = React.useMemo(
() =>
React.forwardRef(function Link(itemProps, ref) {
return <RouterLink to={to} ref={ref} {...itemProps} role={undefined} />;
}),
[to],
);

return (
<li>
<ListItem button component={renderLink}>
<ListItem button component={Link} to={to}>
{icon ? <ListItemIcon>{icon}</ListItemIcon> : null}
<ListItemText primary={primary} />
</ListItem>
Expand Down
20 changes: 8 additions & 12 deletions docs/data/material/guides/routing/ListRouter.tsx
View file
Open in desktop
Expand Up @@ -38,23 +38,19 @@ interface ListItemLinkProps {
to: string;
}

const Link = React.forwardRef<HTMLAnchorElement, RouterLinkProps>(function Link(
itemProps,
ref,
) {
return <RouterLink ref={ref} {...itemProps} role={undefined} />;
});

function ListItemLink(props: ListItemLinkProps) {
const { icon, primary, to } = props;

const renderLink = React.useMemo(
() =>
React.forwardRef<HTMLAnchorElement, Omit<RouterLinkProps, 'to'>>(function Link(
itemProps,
ref,
) {
return <RouterLink to={to} ref={ref} {...itemProps} role={undefined} />;
}),
[to],
);

return (
<li>
<ListItem button component={renderLink}>
<ListItem button component={Link} to={to}>
{icon ? <ListItemIcon>{icon}</ListItemIcon> : null}
<ListItemText primary={primary} />
</ListItem>
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/BadgeUnstyled/BadgeUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ const Badge = React.forwardRef(function Badge(

const styledBadge = <BadgeUnstyled components={{ Root, Badge }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;
Comment on lines +26 to 27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;
const PolymorphicComponentTest = () => {

Would this be better? I know that it produces no difference here since we are not using polymorphicComponentTest but it seems like a component here (or polymorphicComponentTest is not needed at all).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually keep things specific to a test in its scope before hoisting up. That way other tests don't start to rely on it. Up to you, what you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually keep things specific to a test in its scope before hoisting up

That sounds good to me as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could also disable react/no-unstable-nested-components for the TypeScript tests.

material-ui/.eslintrc.js

Lines 319 to 320 in 101fa0d

files: ['*.spec.tsx', '*.spec.ts'],
rules: {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still "invalid React". Better to write it correct everywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough


return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ function ButtonWithCustomRoot(props: ButtonUnstyledProps) {
return <ButtonUnstyled {...props} components={{ Root: CustomButtonRoot }} />;
}

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/InputUnstyled/InputUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ const InputInput = React.forwardRef(function InputInput(

const styledInput = <InputUnstyled components={{ Root: InputRoot, Input: InputInput }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that PolymorphicComponentTest looks like a component (and therefore would allow a hooks call). But inside of it we're declaring new components which is bad. Since it's not actually a component, we adjust the name to reflect that.

const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/MenuItemUnstyled/MenuItemUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import { expectType } from '@mui/types';
import MenuItemUnstyled from '@mui/base/MenuItemUnstyled';

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/MenuUnstyled/MenuUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import { expectType } from '@mui/types';
import MenuUnstyled from '@mui/base/MenuUnstyled';

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/ModalUnstyled/ModalUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ const styledModal = (
</ModalUnstyled>
);

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/MultiSelectUnstyled/MultiSelectUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -77,7 +77,7 @@ const MultiSelectUnstyledComponentsOverridesUsingHostComponentTest = (
/>
);

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/OptionGroupUnstyled/OptionGroupUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -29,7 +29,7 @@ const List = React.forwardRef(function List(

const option = <OptionGroupUnstyled components={{ Root, Label, List }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/OptionUnstyled/OptionUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@ const Root = React.forwardRef(function Root<TValue>(

const option = <OptionUnstyled value={null} components={{ Root }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/PopperUnstyled/PopperUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ function Root(props: PopperUnstyledRootSlotProps) {

const styledPopper = <PopperUnstyled components={{ Root }} open />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/SelectUnstyled/SelectUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -77,7 +77,7 @@ const SelectUnstyledComponentsOverridesUsingHostComponentTest = (
/>
);

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/SliderUnstyled/SliderUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -77,7 +77,7 @@ const Input = React.forwardRef(function Input(

const styledSlider = <SliderUnstyled components={{ Root, Track, Rail, Thumb, Mark, MarkLabel }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/SwitchUnstyled/SwitchUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -41,7 +41,7 @@ const Track = React.forwardRef(function Track(

const styledSwitch = <SwitchUnstyled components={{ Root, Thumb, Track, Input }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/TabPanelUnstyled/TabPanelUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ function Root(props: TabPanelUnstyledRootSlotProps) {

const styledTabPanel = <TabPanelUnstyled components={{ Root }} value={0} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/TabUnstyled/TabUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ function Root(props: TabUnstyledRootSlotProps) {

const styledTab = <TabUnstyled components={{ Root }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/TablePaginationUnstyled/TablePaginationActionsUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@ const styledTablePaginationActions = (
/>
);

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

const requiredProps = {
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/TabsListUnstyled/TabsListUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ function Root(props: TabsListUnstyledRootSlotProps) {

const styledTabsList = <TabsListUnstyled components={{ Root }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/TabsUnstyled/TabsUnstyled.spec.tsx
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ function Root(props: TabsUnstyledRootSlotProps) {

const styledTabs = <TabsUnstyled components={{ Root }} />;

const PolymorphicComponentTest = () => {
const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> = () => <div />;

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material-next/src/Button/Button.spec.tsx
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ const TestOverride = React.forwardRef<HTMLDivElement, { x?: number }>((props, re

const FakeIcon = <div>Icon</div>;

const ButtonTest = () => (
const buttonTest = () => (
<div>
<Button>I am a button!</Button>
<Button color="inherit">Contrast</Button>
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/Button/Button.spec.tsx
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ const TestOverride = React.forwardRef<HTMLDivElement, { x?: number }>((props, re

const FakeIcon = () => <div>Icon</div>;

const ButtonTest = () => (
const buttonTest = () => (
<div>
<Button>I am a button!</Button>
<Button color="inherit">Contrast</Button>
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/DialogContentText/DialogContentText.spec.tsx
View file
Open in desktop
@@ -1,7 +1,7 @@
import * as React from 'react';
import { DialogContentText } from '@mui/material';

const DialogContentTextTest = () => {
const dialogContentTextTest = () => {
const CustomComponent: React.FC<{ prop1: string; prop2: number }> = () => <div />;
return (
<div>
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/Typography/typography.spec.tsx
View file
Open in desktop
@@ -1,7 +1,7 @@
import * as React from 'react';
import { Typography } from '@mui/material';

const TypographyTest = () => {
const typographyTest = () => {
const CustomComponent: React.FC<{ prop1: string; prop2: number }> = () => <div />;
return (
<div>
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-material/test/typescript/components.spec.tsx
View file
Open in desktop
Expand Up @@ -167,7 +167,7 @@ const BottomNavigationTest = () => {
);
};

const IconButtonTest = () => (
const iconButtonTest = () => (
<div>
<IconButton aria-label="delete">
<FakeIcon />
Expand All @@ -187,7 +187,7 @@ const IconButtonTest = () => (
</div>
);

const IconButtonAsLinkTest = () => {
const iconButtonAsLinkTest = () => {
const ForwardedLink = React.forwardRef<HTMLAnchorElement, ReactRouterLinkProps>((props, ref) => (
<ReactRouterLink {...props} ref={ref} />
));
Expand Down