Deconstructing function arguments violates no-unused-prop-types rule

[arturluik]

👋

A similar issue has been reported and got fixed. I stumbled across a variation of #2689 that still does not seem to work.

Versions
eslint 7.22.0
eslint-plugin-react 7.21.5

    const DeactivatePackage = React.useCallback(
        ({context, reloadData}: {context: {id: number; is_active: number}; reloadData: () => Promise<void>}) => {
            if (context.is_active === 0) {
                return <Box />;
            }
            return (
                <Box display="flex" justifyContent="center" color="red">
                    <Button
                        key="release"
                        size="small"
                        aria-label={i18n("deactivate")}
                        onClick={openConfirm(context.id, reloadData)}
                    >
                        <Typography variant="caption">
                            <FormattedMessage id="deactivate" />
                        </Typography>
                    </Button>
                </Box>
            );
        },
        [openConfirm, i18n],
    );

  65:34  warning  'context' PropType is defined but prop is never used     react/no-unused-prop-types
  65:76  warning  'reloadData' PropType is defined but prop is never used  react/no-unused-prop-types

[ljharb]

I'm a bit confused. useCallback is for a callback function, not a component. Thus, DeactivatePackage here should both a) not need .propTypes (which may be the bug), but also b) shouldn't be using useCallback whatsoever. This should instead be a real component defined in module scope.

[ljharb]

A PR to improve component detection so this isn't considered a component would be welcome.

[ljharb]

I'm going to close this since useCallback is not meant for components (that's React.memo).

Happy to reopen if there's a PR with failing test cases.

[baseten]

If you are working with a component that accepts render props then you could well need to use useCallback and return JSX from it. eg. if your render callback relies on a variable from the parent component's closure:

// This is a random abstract example, but can provide a minimal code repro if necessary
type RenderPropParams = {
  // this triggers `react/no-unused-prop-types`
  foo: number;
  bar: number;
}

const ParentComponent = () => {
  const [baz, setBaz] = useState(false);

  const childRenderProp = useCallback(
    ({ foo, bar }: RenderPropParams) => <SomeOtherComponent foo={foo} bar={bar} baz={baz} />,
    [baz],
  );

  return <ChildComponent renderProp={childRenderProp} />;
};

The kind of ugly workaround is this:

type RenderPropParams = {
  // this triggers `react/no-unused-prop-types`
  foo: number;
  bar: number;
}

const ParentComponent = () => {
  const [baz, setBaz] = useState(false);

  const childRenderProp = useMemo(
    () =>
      ({ foo, bar }: RenderPropParams) => (
        <SomeOtherComponent foo={foo} bar={bar} baz={baz} />
      ),
    [baz],
  );

  return <ChildComponent renderProp={childRenderProp} />;
};
};

[ljharb]

@baseten better would be to define a component at module level, and take the baz as a prop to that.

[baseten]

@ljharb

better would be to define a component at module level, and take the baz as a prop to that.

Would you mind giving an example? Part of the problem in our case is that ChildComponent is a third party library so we can't change the API.

[ljharb]

ahh i see what you mean. because it's forced to take a render prop, you indeed need to do what you're doing there. However, since the useMemo argument is just a render prop, not a component, this plugin shouldn't be checking RenderPropParams at all.

I'll reopen this; @baseten, a PR with a failing test case would be most helpful.

[baseten]

However, since the useMemo argument is just a render prop, not a component, this plugin shouldn't be checking RenderPropParams at all.

In the useMemo case, there's no eslint error, that's the workaround to avoid an error instead of using useCallback. It would be nicer to use useCallback though, I'm hoping you'll agree there's a legitimate case for using it and therefore not triggering the eslint error 😄

a PR with a failing test case would be most helpful.

I'll see what I can do 👍

[ljharb]

It shouldn't warn with useCallback either, since that's not a component.