linter: eslint-plugin-react-hooks(rules-of-hooks) fires when it doesn't in ESLint for a wrapped component

[bensaufley]

What version of Oxlint are you using?

0.9.10

What command did you run?

oxlint -c .oxlintrc.json -- path/to/example.tsx

What does your .oxlint.json config file look like?

{
  "rules": {
    "react-hooks/rules-of-hooks": "error"
  }
}

What happened?

A functioncomponent declared inside a component factory does not register as a component for the purpose of linting rules-of-hooks. Given a very limited reproduction like this (please ignore the particular utility of the example):

import { type ComponentType, type ReactNode, useState } from 'react';

interface Options {
  property: string;
}

type MadeComponent<P> = ComponentType<P> & {
  property: string;
};

const makeComponent = <P,>(options: Options, Component: (props: P) => ReactNode) => {
  const Made = Component as MadeComponent<P>;
  Made.property = options.property;

  return Made;
};

const MyComponent = makeComponent({ property: 'foo' }, () => {
  const [value, setValue] = useState('');

  return <input type="text" value={value} onChange={({ currentTarget: { value: v } }) => setValue(v)} />;
});

The call to useState produces the following error in oxlint:

React Hook "useState" cannot be called inside a callback. React Hooks must be called in a React function component or a custom React Hook function. [Error]

This does not occur in ESlint with the same configuration and the same code. Here's a minimal .eslintrc.json, with ESLint 8.56.0 (have not upgraded yet—was investigating oxlint as an alternative):

{
  "plugins": ["@typescript-eslint", "react-hooks"],
  "parser": "@typescript-eslint/parser",
  "rules": {
    "react-hooks/rules-of-hooks": "error"
  },
  "settings": {
    "import/parsers": {
      "@typescript-eslint/parser": [".ts", ".tsx"]
    },
    "react": {
      "pragma": "h",
      "version": "18.2.0"
    }
  },
  "parserOptions": {
    "tsconfigRootDir": ".",
    "project": "./tsconfig.eslint.json"
  }
}

[taearls]

if no one has started working on this issue, can I take a stab at it?

[taearls]

I did some digging into the source code of the react hooks eslint plugin. It appears that they do not currently support this use case, but they might in the future.

https://github.com/facebook/react/blob/main/packages%2Feslint-plugin-react-hooks%2F__tests__%2FESLintRulesOfHooks-test.js#L934-L947

Currently, the react hooks eslint plugin only allows React hooks in function components and in other custom React hooks, which aligns with the current behavior of oxlint.

That being said, I'm not sure what the priorities are in terms of version compatibility. Does it make sense to implement a change for a use case that used to be supported, isn't supported now, but might be in the future?

@Boshen @camc314

[Boshen]

Thank you for the investigation. Let's align to eslint-plugin-react-hooks to remove ambiguity.

Close as won't fix.

[bensaufley]

@Boshen @taearls that isn't true, though. You can use this same code in ESLint and it will not throw a violation. I don't know what to tell you beyond that, but this is not feature parity. You can do this test yourself. useState is not a violation with ESLint, but it is with Oxlint

[bensaufley]

The examples linked to by @taearls are not the same as the example in this Issue

[bensaufley]

As an example, I just pulled the react repo and added this test case:

    {
      code: normalizeIndent`
        // Valid because components can call functions.
        const MyComponent = makeComponent(function () {
          useHook();
        });
      `,
    },

And it passes.

[bensaufley]

It seems like these existing tests are more or less doing the same thing, although the way they're commented makes it seem like they're special exception cases for memo/forwardRef: https://github.com/facebook/react/blob/9daabc0bf97805be23f6131be4d84d063a3ff446/packages/esli
nt-plugin-react-hooks/tests/ESLintRulesOfHooks-test.js#L181-L230

EDIT: indeed there is: https://github.com/facebook/react/blob/9daabc0bf97805be23f6131be4d84d063a3ff446/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#L52-L85

[bensaufley]

OK! This fails:

    {
      code: normalizeIndent`
        // Valid because components can call functions.
        const MyComponent = makeComponent(function wrappedFunc() {
          useHook();
        });
      `,
    },

It looks like isInsideComponentOrHook searches up the tree for the first named function and checks if the name is PascalCase (for components) or useX (for hooks). So the anonymous function passes, but if I name the function it fails.

[taearls]

Ah I see. My mistake. I'll take another look into this, then. Sorry about that.

[camc314]

so tldr is this right?

Should Pass >>

const MyComponent = makeComponent(() => {
    useHook();
});

Should Pass >>

const MyComponent2 = makeComponent(function () {
    useHook();
});

Shuold Pass >>

const MyComponent4 = makeComponent(function InnerComponent() {
    useHook();
  });

Should Fail >>

const MyComponent3 = makeComponent(function foo () {
    useHook();
});

or at least that's the behaviour i see in eslint?

[bensaufley]

Yes that seems correct to me.

[bensaufley]

I imagine this should also pass but I haven't done it myself:

const MyComponent4 = makeComponent(function InnerComponent() {
  useHook();
});

[camc314]

thanks @bensaufley i checked that - you're right. i updated the tldr

[bensaufley]

Probably should fail?

const foo = makeComponent(() => {
  useHook();
});

That might be covered by other tests etc, but just being thorough

[camc314]

@bensaufley this currently passes in eslint so i think we should leave making that one fail out of scope for now.

[bensaufley]

Wow, interesting! That surprises me. But yeah def out of scope for parity