Theo's Typesafe Cult•2y ago

Is useEffect hook good place to manipulate DOM?

Have been developing a multiple state components, was wondering is useEffect hook is good place to add css classes for elements?

export const Input: React.FC<InputProps> = (props) => {
  const { label, name, error, ...restProps } = props;
  const inputElement = useRef(null);

  useEffect(() => {
    inputElement.current.classList.remove(`${styles.inputError}`);

    if (error) {
      inputElement.current.classList.add(`${styles.inputError}`);
    }
  }, [error]);

  return (
    <div className={styles.inputGroup}>
      <label className={styles.label} htmlFor={name}>
        {label}
      </label>
      <input
        ref={inputElement}
        className={styles.input}
        id={name}
        name={name}
        {...restProps}
      />
      {error && <p>{error}</p>}
    </div>
  );
};

export const Input: React.FC<InputProps> = (props) => {
  const { label, name, error, ...restProps } = props;
  const inputElement = useRef(null);

  useEffect(() => {
    inputElement.current.classList.remove(`${styles.inputError}`);

    if (error) {
      inputElement.current.classList.add(`${styles.inputError}`);
    }
  }, [error]);

  return (
    <div className={styles.inputGroup}>
      <label className={styles.label} htmlFor={name}>
        {label}
      </label>
      <input
        ref={inputElement}
        className={styles.input}
        id={name}
        name={name}
        {...restProps}
      />
      {error && <p>{error}</p>}
    </div>
  );
};

10 Replies

tom0619•2y ago

Personally I wouldn't approve this PR. It's not within the 'spirit' of react and the useEffect is completely unnecessary. You could just add a ternary to the input classname

cje•2y ago

useEffect is an escape hatch. avoid it unless absolutely necessary my preferred way to conditionally add classes is with with classnames or clsx https://github.com/lukeed/clsx you can do something like className={clsx(styles.input, error && styles.inputError)} in general you should try to write declarative code over imperative code in react where at all positive

Leonidas•2y ago

Thats indeed the cleanest way imo The object syntax is also very readable for conditional classnames (especially with more than 1 conditional className) classnames(styles.input, { [styles.inputError]: error !== null })

YiaomaOP•2y ago

Thank you everyone for replies! Will this approach work with components relying on 3 - 4 state variables (loading, error, warning, success)

Leonidas•2y ago

Surely, simply add all the conditional classnames in the object as a new key (classname) value (conditional) pair

cje•2y ago

if there's too much stuff going on you might want to start thinking about if there's a better way to slice your logic up but basically yes

Leonidas•2y ago

classnames(styles.input, { [styles.inputError]: error !== null, [styles.loadingStyles]: isLoading })

robotkutya•2y ago

there is also this https://github.com/joe-bell/cva

GitHub

GitHub - joe-bell/cva: Class Variance Authority

Class Variance Authority. Contribute to joe-bell/cva development by creating an account on GitHub.

robotkutya•2y ago

small thing, unrelated, but I'd also ditch the React.FC<Props> typing in favor of just inferring the type, like so

type FooProps = { ... }

const Foo = ({ ... }: FooProps) => {
 // state

 // side effects

 // early returns e.g. loading state

 // return
}

type FooProps = { ... }

const Foo = ({ ... }: FooProps) => {
 // state

 // side effects

 // early returns e.g. loading state

 // return
}

robotkutya•2y ago

https://react-typescript-cheatsheet.netlify.app/docs/basic/getting-started/function_components/

Function Components | React TypeScript Cheatsheets

These can be written as normal functions that take a props argument and return a JSX element.

Gaming

Programming

Is useEffect hook good place to manipulate DOM?