Justin Grant
Justin Grant

Reputation: 46673

React hooks dependency array vs. TypeScript discriminated unions

My React+TypeScript app has a component whose props type is a discriminated union. This helps prevent invalid combinations of props at compile time. But if I need to pass some of those props into a React Hook's dependency list, what's the right way to do it? The Rules of Hooks say that I can't call a hook inside a conditional, but the discriminated props will cause TS compiler errors if they're not inside a conditional (aka type guard).

I've considered the following workarounds, all of which seem like they'll make my code less readable and/or less safe:

  1. Split into multiple components and pushing the hooks into each implementation. This would duplicate a lot of code.
  2. Pulling out each discriminated prop into a local variable, e.g. const values = p.isMulti === true && p.values and using that in the dependency list. In addition to seeming unnecessarily verbose, this approach makes it possible for later code to bypass TypeScript's type checking.
  3. In the dependency list, replace plain property access (e.g. p.value) with a type-safe expression, e.g. p.isMulti === true && p.values. The problem here is that React's exhaustive-dependency ESLint rules complain ("React Hook React.useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.") because these expressions aren't statically testable. I'd have to disable the exhaustive-dep rule which I'd like to avoid because later problems won't be caught by ESLint.
  4. Add a type cast in the dependency list to remove the TS error. This triggers the same exhaustive-deps complaint from ESLint as the type-guard expression above.
  5. Alter the discriminated union to make all props show up in both branches but require the "wrong" branch's value to always be readonly and undefined. This would solve the hooks problem but would make the component somewhat less type-safe.
  6. Refactor the component to remove the discriminator prop and instead push down the union-izing into each prop. This would be easy in the contrived example below (e.g. a single values prop that could either be a string or string[]) but in my real app the union is much more complicated so this "merge props" option would make the component harder to use properly.
  7. Apply a type cast to the props in the deps list, e.g. (p.value as string | undefined. This seems like the least objectionable option at this point.

Is there a better option?

Here's a contrived example that illustrates the problem: https://codesandbox.io/s/interesting-tereshkova-4siy4. Code is duplicated below.

import * as React from 'react';
import { render } from 'react-dom';

type Props =
  | {
      isMulti: true;
      values: string[];
    }
  | {
      isMulti: false;
      value: string;
    };

const X = (p: Props) => {
  return React.useMemo(
    () => (
      <div>
        value(s):
        {p.isMulti === true && p.values.join(',')}
        {p.isMulti === false && p.value}
      </div>
    ),
    [p.isMulti, p.values, p.value], // TS errors on p.value/p.values
  );
};

function App() {
  return (
    <div className="App">
      <X isMulti={false} value={'foo'} />
      <X isMulti={true} values={['foo', 'bar', 'baz']} />
    </div>
  );
}

const rootElement = document.getElementById('root');
render(<App />, rootElement);

Upvotes: 4

Views: 2409

Answers (1)

Badashi
Badashi

Reputation: 1170

The issue typescript sees here is that p.values and p.value are mutually exclusive by your type definition. Instead, consider the following typing:

type Props =
  | {
      isMulti: true;
      value: undefined
      values: string[];
    }
  | {
      isMulti: false;
      value: string;
      values: undefined
    };

Like this, TS will infer that value/values always exist as either their typing or as undefined(which is the default for a property that doesn't exist anyway) instead of being mutually exclusive. TS also infers their typings properly in the sense that if isMulti is true, value will be undefined as expected, for example.

An alternative would be to forgo the whole value/values aspect:

type Props =
  | {
      isMulti: true;
      value: string[]
    }
  | {
      isMulti: false;
      value: string;
    };

in this case, you could just use value instead of having two attributes with similar names.

Upvotes: 1

Related Questions