Common mistakes writing React components with hooks (lorenzweiss.de)

Wikipedia says it's both https://en.wikipedia.org/wiki/React_(web_framework)

I think this is an important nitpick though in your defence. I have just started learning React having become pretty competent with Angular (day job plus some hybrid apps) and I have to say that I'm struggling to understand its popularity compared to Angular.

Angular feels fully fleshed out, adheres to MVC mostly and has nice separation of html, css and the UI TS code. I come from a native coding background so this feels logical and I can jump into a new project and get my bearings pretty quickly.

React on the other hand seems to be jQuery gone mad with CSS, JS and pseudo HTML all mixed together. Ternary statements in JSX is awful to look at.

To me it seems React is solely there to solve the V in MVC and then with a host of additional libraries (of the users choice) some kind of MVC system can be cobbled together if needs be.

I'm not saying React is bad, clearly it is very popular and has it's great use cases, but I don't think it's Angular vs React.

>> but React is not a framework

It's a framework which advertises itself as a library.

In reality, there is nothing library-like about bypassing the DOM for rendering and making developers use a custom programming language (JSX)... and please don't give me this tired old argument that JSX is not compulsory! Has anyone ever seen a real React application which does not use JSX? Exactly.

If 99.9999% of developers are using React as a framework, then for all general communication purposes, React is a framework.

I think React is both and when you describe it as either will depend on how you're using it. If you have a website with a couple of React components here and there, then you have React the library. If you have your app setup as a SPA, with react router, redux, react forms, maybe you started the app with createReactApp, etc, then you are using React the framework. I don't see how you can argue it's not a framework when every single piece of your app is a react specific piece. It's no longer just a view layer, it's an ecosystem of modules whose common ground is to turn React into a framework.

What is difference between a front-end library and a front-end framework?

Moment.js is a library, you can use in any part of your app and it doesn't define how your application is going to work. You are in charge of the flow of the application. React pretty much defines how your application is going to work from the beginning. It can call itself a "library" but by any practical means, it isn't.

React's documentation calls it a library but we don't have to. I think the reason people go back and forth on this is that there are a couple definitions of framework out there. A lot of people consider it to be a sort of continuum, where a library becomes framework-like as it adds more and more functionality. I understand that framework can sometimes connote bloat, which is probably why the React docs avoid the term. And I think it's perfectly reasonable to say that frameworks are a subset of libraries, so perhaps they're not strictly wrong to describe React as a library.

But, as others have pointed out in this thread, a much more useful distinction is where the library's code gets called in the stack. If your code is at the top of the call stack with library code below it, then it's a framework. If their code is at the top, then it's a library. In short, you call a library, a framework calls you (inversion of control). So in that sense React is most definitely a framework.

Also, I'm willing to be wrong on this one if somebody can give me a meaningful and objective reason that React should be considered a library but Angular should be considered a framework.

yes seems also wrong to me. not sure what linter would complain here, that would depend a little bit what fetchData & updateBreadcrumbs really use and do.

a much bigger problem with code like this is the fetching of data gets much more complicated when the second parameter of useEffect is not just an empty array

Can you explain what you mean by that? I'm honestly a little lost on what you're saying but I am curious.

Exemples would be welcome here as I've a strong belief that hooks made react components cleaner / shorter

I haven't use hooks and don't prefer it, however I must say that there isn't alternative to hooks that is shorter in syntax, and maybe easier.

I get what you're saying but disagree. Hooks are essential and make life much easier for the react developer, especially when using something like `react-redux` or `react-router`. Prop-drilling or HoC might seem like a better design until you actually have to work in a system that leverages them and realize it's an indirection nightmare.

While I don't agree that writing a component with a hook is always a mistake, I have seen them used in situations where it wasn't warranted.

In the end, they are just another tool -- knowing how to use the tool appropriately is the critical part.

Yeah I felt the same way about the first one.

Yeah we're preventing re-renders there but what are we really preventing / saving, nothing significant?

Anytime I run into a situation where I didn't like how / when a component was re-rendering, it absolutely was not because there was a random bit of state like a counter was in state that didn't need to be... usually it was just a more complex situation unfolding.

It's a good illustrative example, but not a 'common mistake' IMO.

To add to this, the React docs state you should never rely on renders for side effects. In dev mode, React will even render your components twice to suss out any render relying side effect bugs. Your component should always work the same whether it's rendered once or rendered a million times.

> The first one just feels like a premature optimization

It is in a browser context, but less with React Native which relies way more on refs. It also never hurts (imho) to explain why refs exist and why/where to use them as they can easily be abused (and often are) by devs trying to replicate OOP patterns in React.

Yes. The author should replace "This is dangerous" with "This is a tiny bit sub-optimal" in the first example.

EDIT: "This is dangerous" is also the wrong label for the 2nd example. Should be "This is not the cleanest way"

EDIT2: "This is dangerous" is also the wrong label for the 3rd example. Should be "This is not the most readable". I'd also note that I'm surprised the author has seen this mistake a lot; the "solution" looks like the happy path most people would follow in the first place.

EDIT3: "This is dangerous" in the 4th example should be replaced with "This could be simplified"

EDIT4: "This is dangerous" in the 5th example should be "This makes redundant API calls". I'd also note that I'm surprised this example is even included. How could anyone miss that fetchData() is being called every time updateBreadcrumbs() is called?

I have a really large chat app with thousands of users and I realized the other day that I accidentally re-render the root component on _every keystroke_. Still, it's not a disaster and I haven't bothered to fix it because no-one notices.

There is no harm in adding something to the dependency array that you don't expect to ever change. The linter is your friend; make its job easier.

I mean, onSuccess could change. And as you suggested, creating the extra callback is redundant.

  const fetchData = () => {
    setLoading(true);
    callApi()
      .then((fetchedData) => {
        setData(fetchedData);
        onSuccess();
      })
      .catch((err) => setError(err))
      .finally(() => setLoading(false));
  };
  useEffect(() => {
    fetchData();
  }, []);

becomes

  useEffect(() => {
    setLoading(true);
    callApi()
      .then((fetchedData) => {
        setData(fetchedData);
        onSuccess();
      })
      .catch((err) => setError(err))
      .finally(() => setLoading(false));
  }, [onSuccess]);

Same with the empty array

I would ignore whoever told you not to use an empty array for the dependency argument. Change your linter rules. It's fine.

If you need an effect to run before the component draws, try `useLayoutEffect` instead of `useEffect`. https://reactjs.org/docs/hooks-reference.html#uselayouteffec...

  function useFirstRenderComplete() {
    const renderCountRef = useRef(0);

    useEffect(() => {
      if (renderCountRef.current) {
        return;
      }

      renderCountRef.current += 1;
    }, []);

    return renderCountRef.current !== 0;
  }

If the dependency array lint rule is yelling at you there's a very, very good chance your component is broken. As an example I used to see class components like this all the time:

   class Foo extends React.Component {
      componentDidMount() {
         fetchFoo(this.props.id)
            .then(foo => this.setState({foo}))
      }
      render() { ... }
   }

This component is fundamentally broken because there is no `componentDidUpdate` to re-call `fetchFoo` when `this.props.id` changes. I could easily be rendering a new id with an old id's foo. The developer is making an assumption that the initial value of `id` will never change, therefore making assumptions about how the parent component is instantiating `Foo`. Rewriting this using hooks makes this immediately obvious:

   const Foo = ({id}) => {
      const [foo, setFoo] = React.useState(null)
      React.useEfect(() => {
         fetchFoo(id).then(setFoo)
      , []) // Linter complains, and it should!
      return ...
   }

There's nothing wrong with using the empty array as your deps so long as it's actually what you want to do. If `fetchFoo()` didn't require any arguments and we only wanted to invoke it once [] would absolutely be the right dependency array. There are few cases I can think of where disabling the linter check is correct. The most common occurence in my code is if the effect uses a value not required to be fresh: e.g. something used for an optimization.

I found this article incredibly helpful in answering these questions: https://overreacted.io/a-complete-guide-to-useeffect/

I just inherited a React project that was full of these mistakes. The code was littered with "onClick" and "history.push". They used MobX because they wanted a store, even though they didn't need it. Components would often receive this store as a prop, instead of just the values it needed to render.

So yes, these mistakes are very common. There's just quite a few ways to do things, and if you don't do any research, they all have the same outcome.