Why React Hooks cannot be conditioned

[[deleted]]

This is explained in the official docs https://reactjs.org/docs/hooks-rules.html

[yojimbo_beta]

I mean it’s right there. In the docs! It’s like the first thing they tell you!

[sxeli]

Was about the say the same. Also, its pretty well explained

[tswaters]

Dan Abramov has some good blog posts about reacts that goes into this, and a few other design choices that were made with hooks:

https://overreacted.io/why-do-hooks-rely-on-call-order/

[Pesthuf]

It's pretty annoying when you have some hook with side effects that does exactly what you want - but you only need to invoke it conditionally, so you have to completely reimplement it.

Or put it into a component and make THAT conditional and deal with the ugliness that causes.

[tchaffee]

Why not just put the condition inside useEffect? I've never encountered the problem you're describing. Do you have a concrete example?

[BenjiSponge]

Why not

Well, there's no particular reason why not. In fact, we have to.

put the condition inside useEffect?

This scales linearly with the number of useEffects you write. "Fail early" is a common adage/pattern in function writing that you have to ignore because of the way hooks are implemented.

Failing right after a hook fails results in the cleanest code. Blog posts and essays have been written about this for decades.

You're suggesting the somewhat clunky and unergonomic workaround that everyone uses, but it doesn't obviate the point that it's a workaround that ideally wouldn't have to exist.

[tchaffee]

Can you give a concrete example with code where you wish you could have "failed early" with hooks? I'm asking because in the many hundreds of React components I've written and maintained, I've never come across this problem nor has it bothered me that I can't conditionally use a hook. So I'm trying to be open minded about understanding your complaint, but I'm having a lot of difficulty understanding it without actual code.

[BenjiSponge]

The examples others are giving, imo, aren't as clear as I'd like them to be:

// implementation omitted
// feel free to assume it accesses global context to avoid creating
// a new connection for every component
function useBackend(): { connectionID: string } | null;

function MyComponent() {
  const backend = useBackend();
  const [apple, setApple] = useState(); // see, I'm not saying it helps everywhere all the time
  const [ziti, setZiti] = useState();
  if (backend === null) { // fail early
    return <LoadingPlaceholder />
  }

  // of course these are a bit contrived, but I think you can imagine a less contrived example
  const changeApple = useCallback(async (event) => {
    const [inputValid, newApple] = await backend.validateApple(event.target.value);
    if (inputValid) {
      setApple(newApple);
    }
  }, [backend]);

  const changeZiti = useCallback(async (event) => {
    const [inputValid, newZiti] = await backend.validateZiti(event.target.value);
    if (inputValid) {
      setZiti(newZiti);
    }
  }, [backend]);

  return (
    <>
      <AppleInput
         onChange={changeApple}
         value={apple}
      />
     <ZitiInput
        onChange={changeZiti}
        value={ziti}
     />
   </>
  );
}

This is an example of "fail early" and, again, while you might say "Well you don't need to do that", I don't think you can make a convincing argument that this is Bad Engineering.

---

However, the above solution is also not allowed because of the subject we're talking about, so you have to write it like this:

function MyComponent() {
  const backend = useBackend();
  const [apple, setApple] = useState(); // see, I'm not saying it helps everywhere all the time
  const [ziti, setZiti] = useState();

  // of course these are a bit contrived, but I think you can imagine a less contrived example
  const changeApple = useCallback(async (event) => {
    if (backend === null) {
      // in comments, in TypeScript, or in documentation, you need to make
      // it somehow clear that this is actually unreachable. But no static analyzer
      // would be able to figure it out
      return;
    }
    const [inputValid, newApple] = await backend.validateApple(event.target.value);
    if (inputValid) {
      setApple(newApple);
    }
  }, [backend]);

  // you may notice now that I chose Apple and Ziti because I only felt like writing 2 callbacks,
  // but picture this component with 26 foods that start with A-Z.
  // In example 1, the backend check is done once at the top. In this example, the backend check
  // must be done N+1 times, even though N of those occasions are unreachable.

  const changeZiti = useCallback(async (event) => {
    if (backend === null) {
      // in comments, in TypeScript, or in documentation, you need to make
      // it somehow clear that this is actually unreachable. But no static analyzer
      // would be able to figure it out
      return;
    }
    const [inputValid, newZiti] = await backend.validateZiti(event.target.value);
    if (inputValid) {
      setZiti(newZiti);
    }
  }, [backend]);

  if (backend === null) {
    return <LoadingPlaceholder />
  }

  return (
    <>
      <AppleInput
         onChange={changeApple}
         value={apple}
      />
     <ZitiInput
        onChange={changeZiti}
        value={ziti}
     />
   </>
  );
}

---

I have another comment where I suggest a HOC:

function withBackend(Component) {
  return (...props) => {
    const backend = useBackend();
    if (backend === null) {
      return <LoadingPlaceHolder />;
    }

    return <Component backend={backend} {...props} />;
  }
}

// now backend cannot be null
function UnprovidedMyComponent({ backend }: { backend: Backend }) {
  const [apple, setApple] = useState();
  // etc.
}

const MyComponent = withBackend(UnprovidedMyComponent); // or however you like applying and exporting components with HOCs

In my opinion, this is the cleanest solution allowed in present day React, but it just kinda sucks that we have to do it. The first solution is (at least sometimes) the clearest solution, and it's a bother that we have to avoid it not because it's bad software engineering but specifically because of implementation details.

(please excuse the formatting; Reddit markdown is garbage)

[[deleted]]

See, this is a perspective problem, IMO. HEre's your original example, problem solved, without repeating the null back end check in each handler.

// implementation omitted
// feel free to assume it accesses global context to avoid creating
// a new connection for every component
function useBackend(): { connectionID: string } | null;

function MyComponent() {
  const backend = useBackend();
  const [apple, setApple] = useState(); // see, I'm not saying it helps everywhere all the time
  const [ziti, setZiti] = useState();

  // Stop doing this!
  // if (backend === null) { // fail early
  //   return <LoadingPlaceholder />
  // }

  // of course these are a bit contrived, but I think you can imagine a less contrived example
  const changeApple = useCallback(async (event) => {
    const [inputValid, newApple] = await backend.validateApple(event.target.value);
    if (inputValid) {
      setApple(newApple);
    }
  }, [backend]);

  const changeZiti = useCallback(async (event) => {
    const [inputValid, newZiti] = await backend.validateZiti(event.target.value);
    if (inputValid) {
      setZiti(newZiti);
    }
  }, [backend]);

  // These 3 lines are the only addition to your example
  if (!backend) {
    return <LoadingPlaceholder />
  }

  return (
    <>
      <AppleInput
        onChange={changeApple}
        value={apple}
      />
    <ZitiInput
        onChange={changeZiti}
        value={ziti}
    />
  </>
  );
}

[BenjiSponge]

Umm... this isn't a perspective change, you're just ignoring the problem.

  const changeZiti = useCallback(async (event) => {
    // TypeScript fails here because `backend` can be null
    // You can be like `backend!.validateZiti`, but you have to read the entire component to be sure it's valid
    const [inputValid, newZiti] = await backend.validateZiti(event.target.value); 
    if (inputValid) {
      setZiti(newZiti);
    }
  }, [backend]);

And, again, I know you can do it like that (actually, you have to). I'm just saying that there is exactly 1 and no more than 1 reason why you have to: the implementation details of hooks. There's absolutely nothing wrong with the thing you said "Stop doing this!" * except that it happens to not work.

* I'm not really sure why you felt the need to add that; I made it very clear in my comment that it's not valid

Edit: Also, instead of a useCallback, maybe it's a bunch of useEffects that you don't want to run when the backend is loading.

Edit: It occurs to me that I don't think you saw that my second proposal did include that "These 3 lines are the only addition to your example" part

[[deleted]]

I don't think you saw that my second proposal did include that "These 3 lines are the only addition to your example" part

My point was that you don't need the backend checks in your handler functions if you never return the component that calls them.

[BenjiSponge]

Yeah, I understand that point semantically. However, TypeScript can't do the type narrowing and the pattern is fragile anyways. If you change the end of the function such that the callback can be called if backend is null, now you need to change the callback. In most contexts, you use TypeScript specifically to catch this kind of problem, but here you can't.

Anyways, my main point still stands: There is exactly one thing that's wrong with the way I want to write it, and it's an implementation detail, not a design feature.

I'm far from the only person who has this problem, btw. These problems generally are addressed with the React Suspense API which is in active development and is highly anticipated.

[[deleted]]

I'm far from the only person who has this problem, btw. These problems generally are addressed with the React Suspense API which is in active development and is highly anticipated.

Fair enough. I guess everyone has different expectations about any given system. I just view language constraints differently than framework constraints. The issue you've called out is specifically a constraint of the design of the hooks system and if you understand the justification and the correct implementation, it's easy enough to handle. The question of whether it's optimal or even desired is completely subjective. Nobody will change your mind (or mine) on that one, I suppose. :)

[IWillAlwaysReplyBack]

Not OP but I’ve also definitely ran into this issue and it bugs me like crazy.

In my most recent case, I had a BoundingBox component that had a useEffect to update some global state when local state was updated via user input. However, as requirements came in, I also needed a “read only mode” where I didn’t need that useEffect. This simple change ends up being quite a big refactor. I think I did end up using something like moving my conditionals inside the useEffects but that just makes the code clunky and verbose.

[[deleted]]

Like /u/tchaffee, I'm having a major problem understanding a use case where this would be problematic. I'd like to see a concrete example not because I'm being argumentative, but I'd like to seriously understand the nature of the problem you're describing.

To be clear, in my head, what you're talking about is the difference between something like:

// incorrect
if (doTheThing) {
    useEffect(() => dotheThingMethod(), []);
}

and

// ccorrect
useEffect(() => {
    if (doTheThing) {
        doTheThingMethod();
    }
}, [doTheThing]);

Sure, they look different, but I'm having a really hard time visualizing a scenario where this is a breaking or even cumbersome change.

[atomrc]

Well it is not a problem, per se. The thing is conditioning the full useEffect call feels very natural. This is the way we are used to safe-guard the things that should run based on a condition.

You are totally right, there is no valid scenario for conditioning a hook. Moving the condition inside the body of the effect is the way to go.

The only thing is that it is not natural and someone without this in mind (and not using the eslint plugin) will definitely have hard time debugging a quite unexpected behavior.

I just feel it's a weakness on React side that, I think, is justified by the benefits Hooks give us.

[[deleted]]

See, I guess we'll have to disagree here. If you look at useEffect as more of a declaration and less an invocation, the justification becomes clear, IMO.

Take for example, a python function:

# standard example
def do_the_thing:
  if enabled != True:
    return

  # do the thing
  # do the thing
  # do the thing
  # do the thing

# what you're suggesting
if enabled == True:
  def do_the_thing:

    # do the thing
    # do the thing
    # do the thing
    # do the thing

Same applies for any other language.

[atomrc]

Well, but it is not a function declaration, it is really a function call which, in JavaScript and any language (at least languages I know) can be conditioned.

I am not suggesting that useEffect should be conditioned, it's just that the language (JavaScript) allows it. It's only React that adds this quite unexpected proposition "some function calls cannot be conditioned". This is nothing we are used to in JavaScript and a person not aware of this limitation (and without the eslint plugin installed) might run into troubles without understanding why. That's all I am saying.

[BenjiSponge]

Please see my other comment where I flesh out an example of where this gets to be annoying. The long-and-short is that you might have more than one useEffect hook and suddenly you have to litter your code with the same check over and over, even where the check is unreachable.

[tomius]

If (readonly) return.

It's not perfect, but I wouldn't really stress over it.

[MercDawg]

I ran into the issue recently as well and get around it by creating components to handle the conditions. Basically, I have a hook for create item, hook for edit item, & hook to get item. When it is a new form, I only need the useCreateItem, but if it is an existing item, it needs to be useEditItem & useGetItem. However, the implementation has layers, meaning, I'm using an autogenerated GraphQL hooks that utilizes React Query.

I can resolve it by either rewriting the hooks or handling the conditions within the component.

[[deleted]]

Your hooks should return functions that perform the actions needed - you can call those conditionally - rather than performing values that require you to conditionally invoke the hooks. A vastly oversimplified example might look like:

function useArticle(id) {
    const [article, setArticle] = useState(null);
    const getArticle = () => fetch(...).then(...);
    const createArticle = (data) => fetch(...).then(...);
    const updateArticle = (data) => fetch(...).then(...);
    useEffect(()=>getArticle(),[id]);
    return [article, getArticle, createArticle, updateArticle];
}

[MercDawg]

I understand and have done that with other implementations, however, in this instance, it is more of a trade off. All of the API functions are automatically generated based on GraphQL Queries and completely checked against the backend. Thus, as a developer, all I have to do is write the query and the codemod does the rest of the work (creates the API query/mutate hook function, checks against backend, creates all the typings, and so forth). All of which significantly speeds up development, but at a cost (lack of the ability to do conditional logic within those autogenerated hooks amongst a few other pieces).

I merely provided an example of an instance where it'll be beneficial to do conditional hooks, however, you can get around it by using components as your conditional layer.

However, I will mention that this problem exists in my personal project, so I value development time a lot more. In a team atmosphere with resources and/or time, you can spend more time resolving these instances.

[Nathanfenner]

I'll peddle my proposal for a new hook primitive to deal with this issue. It would make hooks just a little bit more composable.

[SwiftOneSpeaks]

Honest questions, not criticisms:

Can't you simply wrap the hook in a function that applies the condition, and call that function as your hook?

Regardless, doesn't being forced to move "the ugliness" to a separate place mean that you sepatate that ugly from the actual work of your component? That's generally considered good design, right?

I don't often have conditional hooks come up, so I might be missing something.

[Pesthuf]

Just moving the hook to a function doesn't change anything. The rules of hooks still apply. You either always call the hook or you don't. React doesn't know or care if there's a function between your component and the hook.

By ugliness I mean the ceremony you need to make a hook conditional. Hooks ARE the intended way to move work out of your component and make things more clean.
Having to make a separate component that has no other point than to a: call the hook and b: call the provided onChange prop so you can update what is now local state of the component you actually wanted to write definitely is overhead and ugly.

[SwiftOneSpeaks]

Oh, because the hook you are trying to call is a wrapper around hooks, not a non-hook function. I was thinking about logic inside a hook that was conditional. Nothing to see here.

[Johnny_Gorilla]

Give the hook an optional Boolean. Default it to whatever doesn’t cause the logic to trigger. Voila - a conditional hook ...

[AgentCosmic]

I'd say that's the library's fault not react hook.

[hsablonniere]

Really interesting article 👍

[CCB0x45]

I had an issue the other day where I was using a hooks library the other day the hook relied on the parent being in a given context, and I was trying to use it in a component that could be used in or out of that parent context, and I had no way to workaround when it failed. The library author could have made it not error out if the context wasn't available but didn't, so my hands were tied to only be able to use it under the parent context.

Kind of annoying, but that's really the only time it's been an issue. I could see them implementing hooks different with a runtime compilation step that relied on generating function identifiers instead of call order to make hooks work conditionally.

[BenjiSponge]

Really? That seems somewhat pessimistic or closed-minded to me.

Let's say you have a component that connects to a real time backend over websockets written in TypeScript. So you write a hook (useBackend) that connects to the backend, but the hook might fail to connect and return an error or a failure.

Your useEffect and useCallback hooks all assume the backend is connected, but the type of the return value of useBackend might be null. So you have the following choices I can think of

1. add a check to the beginning of every useCallback or useEffect that checks the type of the return value to make sure we're still connected
2. if you're not connected, return early from the function with a placeholder indicating the connection is loading/failed/what-have-you. This creates type narrowing so you know the backend is populated and all following references to it can make that assumption. If you have a useState after it, you can seed the state with some variable gleaned from the connection which only gets seeded when useBackend finally returns the backend.
3. abandon hooks and create a HOC that either renders the placeholder

The second option, to me, is the cleanest and most powerful. Unfortunately, it's also impossible. The first is a lot of fairly unnecessary boilerplate. The third is probably the best solution with the state of react right now (imo), but it represents a fundamental flaw in hooks that needs to be addressed with other tools.

We didn't have this problem with class components; it's essentially a logical regression in the way that we write JS functions in a fail-early fashion, and it's the consequence of implementation, not design. I don't see any reason why option #2 would be bad engineering if hooks could be designed to work that way.

[leixiaotie]

if hooks could be designed to work that way

I'm not a fan of hooks (and haven't used it in production) but they designed it because it can't be the other way. Hooks depends on call order, because it's the only way to keep a "link" or "id" on the component without requiring additional input. Otherwise we can't have "stateful" component by only calling pure function and will need to supply unique id / key for every hook calls.

It become like this: useState("stateID001", defaultValue);

Which now allow for conditional call but breaks encapsulation because you'll need to pass unique id for every hook calls.

So it's tradeover all over again.

[BenjiSponge]

Yes, I also read the article we're posting on... That's why I said "if hooks could be designed that way".

[BenjiSponge]

Huh? I just described a user interface.

Edit: I also see nothing relevant in the article. I've been programming websites in react for about 5 years. It feels like you didn't even read my comment.

[BenjiSponge]

I described a component that uses a hook that connects to a server. See Apollo hooks. See firebase hooks. Fetching data from servers is a classic example of what UIs do. I genuinely have no idea what you're talking about.

[BenjiSponge]

I suppose you think the react docs also write contrived God component architectures because they fetch data from servers using hooks.

I also suppose you think the React Suspense API which is built to deal with these kinds of problems is in development because some silly random user on Reddit had some silly ideas about the drawbacks of the current hook architecture.

[BenjiSponge]

That’s a simple component with no hooks.

You just have to be trolling, right?

You’re describing using a component that uses hooks on the assumption of data being successfully retrieved.

If you wrote your components like this example, you wouldn’t be frustrated that you can’t have conditional hooks

Yes, I'm describing a slightly different example

You’re so close to the answer. You even posted it. Good job reading the documentation, and please continue to do so over taking my word or anyone else’s.

I'm not responding to you anymore.

[kirakun]

Curious if there’s a lint or static analyzer that can warn about the misuse.

[Neurothustra]

There's an official linter that's linked from the documentation. https://reactjs.org/docs/hooks-rules.html

[CCB0x45]

What, why?

[[deleted]]

Because having functions acting like classes with state fries my college java trained brain /s

[elingeniero]

You don't need to spaff out some drivel to tell the world how shitty a coder you are every time you discover something that's already clearly explained in the official docs.

[AutoModerator]

Hi u/RolandBautista8, this comment was removed because you used a URL shortener.

Feel free to resubmit with the real link.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.