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.

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

The linter should also warn against bad practices inside hooks #34

Open
SRachamim opened this issue Jan 21, 2020 · 7 comments
Open

The linter should also warn against bad practices inside hooks #34

SRachamim opened this issue Jan 21, 2020 · 7 comments

Comments

@SRachamim
Copy link

SRachamim commented Jan 21, 2020

I expect the lint to also work inside React hooks functions:

Every hook function (which is a function named use* should not constantly return a new reference (which is an object/function/array/JSX which is not created outside the hook nor inside a useMemo/useCallback).

@cvazac
Copy link
Owner

cvazac commented Jan 23, 2020

Like this?

const Component = ({prop = []}) => {
 useEffect(()=>{
    //
  }, [prop])
 return <div/>
}

@SRachamim
Copy link
Author

SRachamim commented Jan 23, 2020

Like this:

// should pass
// const val = {};

// should pass
// const setVal = () => /* ... */;

const useSomething = () => {
  // should fail
  // const val = {};

  // should pass
 const val = React.useMemo(() => {}, []);

  // should fail
  // const setVal = () => /* ... */;

  // should pass
  const setVal = React.useCallback(() => /* ... */, []);
  
  return [val, setVal];
};

@cvazac
Copy link
Owner

cvazac commented Oct 29, 2020

It's a great idea. This should definitely be done, and I hope to get to it. Thanks

@jmancherje
Copy link

@cvazac I've run into a very similar situation which I hope can also be solved. Essentially the same issue, but any regular function (non-hook) that returns an object, array or function seems to be a workaround for the linting rules. We implemented the 4 recommended rules and I saw this kind of code slip through.

This is a contrived example but it's essentially what I saw in our codebase after implementing the recommended lint rules.

function Component() {

    const value1 = getChildValue();

    const value2 = {
        foo: 'bar'
    };

    return (
        <>
            <Child value={value1} /> // <-- does not violate linting rule
            <Child value={value2} /> // <-- violates linting rule
        </>
    );
}

function getChildValue() {
    return {
        foo: 'bar'
    }
}

@bolatovumar
Copy link

This would be a useful rule. I notice the pattern of custom hooks returning a new reference every time all too often.

@FPDK
Copy link

FPDK commented Feb 20, 2021

If this is implemented, would it produce a warning or an error?
After reading some articles on the subject, I get the impression that useMemo and useCallback can sometimes be slower than the cost of re-declaring.

@bolatovumar
Copy link

If this is implemented, would it produce a warning or an error?
After reading some articles on the subject, I get the impression that useMemo and useCallback can sometimes be slower than the cost of re-declaring.

@FPDK you can always configure whether something is an error or a warning yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants