Is there an ESLint rule to error/warn to avoid "Mirroring props in state"?

[oscard0m]

Description^*

I would like to know if there is any existing ESLint rule this plugin for preventing mirroring props in state

❌ Do not use a prop as the initial useState() value:

Click to expand!^**

const ParentComponent = () => {
const [initialValue, setInitialValue] = useState("Initial value");

useEffect(() => {
    setTimeout(() => {
      setInitialValue("Changed value");
    }, 1000);
  }, []);

  return <ChildComponent initialValue={initialValue} />;
};

const ChildComponent = ({ initialValue }) => {
  const [inputValue, setInputValue] = useState(initialValue);

  const handleChange = (e) => {
    setInputValue(e.target.value);
  };

  return <input value={inputValue} onChange={handleChange} />;
};

✅ Use useEffect() instead to fix it:

Click to expand!^**

import React, { useState, useEffect } from "react";

const ParentComponent = () => {
  const [initialValue, setInitialValue] = useState("Initial value");

  useEffect(() => {
    setTimeout(() => {
      setInitialValue("Changed value");
    }, 1000);
  }, []);

  return <ChildComponent initialValue={initialValue} />;
};

const ChildComponent = ({ initialValue }) => {
  const [inputValue, setInputValue] = useState("");

  useEffect(() => {
    if (initialValue) {
      setInputValue(initialValue);
    }
  }, [initialValue]);

  const handleChange = (e) => {
    setInputValue(e.target.value);
  };

  return <input value={inputValue} onChange={handleChange} />;
};

Context

It's something React Docs (beta) suggests:

https://beta.reactjs.org/learn/choosing-the-state-structure#avoid-redundant-state -> check Deep Dive box

Notes

• ^*If there is not an existing ESLint rule for this and you think it could have sense I will edit the title and the issue description
• ^**Thanks to @volodymyrhudyma since I used his awesome article example to illustrate the issue.

[ljharb]

The guidance is to avoid conceptually mirroring props in state, which has nothing to do with the actual prop/state names. An eslint rule would only catch the trivial cases where the names were the same - and some of these would be false positives.

[oscard0m]

The guidance is to avoid conceptually mirroring props in state, which has nothing to do with the actual prop/state names. An eslint rule would only catch the trivial cases where the names were the same - and some of these would be false positives.

I see. Maybe it should be React printing a console.error in development mode? What do you think @ljharb ?

[ljharb]

I don't think it is possible to provide a programmatic warning here, since it takes a human to understand the conceptual mirroring - having a prop named "foo" and a state named "foo" in NO way guarantees that you're mirroring props into state; and NOT having those name overlaps also does not guarantee you are NOT mirroring props into state.

I think this is just one of the many things that must be left to human review.

[oscard0m]

I don't think it is possible to provide a programmatic warning here, since it takes a human to understand the conceptual mirroring - having a prop named "foo" and a state named "foo" in NO way guarantees that you're mirroring props into state; and NOT having those name overlaps also does not guarantee you are NOT mirroring props into state.

Thanks for the quick feedback on this @ljharb. I don't have much experience with ASTs/React compiler/ESLint rules so probably I'm missing some parts in my head. For my learning and understanding, these are the questions I have in mind right now for you:

1. Would the logic to apply be simply a prop (one of the keys from props argument) is not used as argument for useState() function? No matter how the props or the states are named, right?
2. Are there other use cases a part from the one mentioned in question 1. you have in mind? In my opinion only hooks solutions should be covered. Since it's a warning I don't we should invest in a retro-compatible warning.
3. In general, what are the limits of ESLint Rules or React error checks? What kind of checks can't be done programatically? Do you know some good resources I can read or check from?

I think this is just one of the many things that must be left to human review.

Sure, if we conclude there is no possibility to check this programmatically I'm closing the issue.

[ljharb]

ahh, maybe i misunderstood.

It's perfectly reasonable to determine state values based on props; that's what the getDerivedStateFromProps class method is for. It seems just as reasonable to use them in a useState/setState hook call.

[oscard0m]

But seems to be a bad practice, right? Or at least something to warn. Here's the screenshot (there's no anchor link, sorry) from React Docs (beta): Don't mirror props in state:

[ljharb]

ok, so i can see an argument for a prop value being used as an initial state value, and having that warn.

Such a rule would not just need to detect it with SFCs and hooks, but also with class and createClass components - anything that isn't using gDSFP.

[oscard0m]

ok, so i can see an argument for a prop value being used as an initial state value, and having that warn.

Such a rule would not just need to detect it with SFCs and hooks, but also with class and createClass components - anything that isn't using gDSFP.

Aham. I see. Then... on your experience, do you think it would be feasible and interesting to lint or too much use-cases to cover?

[ljharb]

I'm not really sure.

It's definitely a good bit of work to make a robust rule for this. It might indeed catch some problem cases, which is great! The error message could include links to the react docs you've linked above.

The question that can't really be answered is whether it'll come up often enough to be worth it, but I still think it's worth a PR.

[oscard0m]

I'm not really sure.

It's definitely a good bit of work to make a robust rule for this. It might indeed catch some problem cases, which is great! The error message could include links to the react docs you've linked above.

The question that can't really be answered is whether it'll come up often enough to be worth it, but I still think it's worth a PR.

I would like to give it a try and draft something and from that draft I guess we can better see effort 🆚 benefit I guess. Would it be possible I take this?

[ljharb]

Go for it!

[pke]

I constantly run into this problem during mentoring (beginners and seasoned devs). And I do not agree that it is called "mirroring". It's initialisation and there a perfect reasonable use cases where the initial prop value is the initial state and later overridden from inside the component.

Lets take this Loading component:

export function Loading({ text = "Loading...", style }: Props) {
  const [currentText, setText] = useState(text)
  
  useEffect(() => {
    const timer = setTimeout(() => setText("Still loading..."), 5000)
    return () => clearTimeout(timer)
  }, [])

  return (
    <View style={[styles.wrapper, style]}>
      <ActivityIndicator size="large" />
      <Text style={styles.text}>{currentText}</Text>
    </View>
  )
}

It's reasonable that this could be used as a global component to display loading state and that its input prop text changes over time. But this would not be reflected in the components output.

So the ESLint rule should check if a prop is directly used to initialise useState and then require to find a matching useEffect with this dependency and a call of said useStates setter function inside the effect hook.

The fix for the above component is easy: It should update the currentText to props.text and also reset the timer when the initial text changes.

export function Loading({ text = "Loading...", style }: Props) {
  const [currentText, setText] = useState(text)
  
  useEffect(() => {
    setText(text)
    const timer = setTimeout(() => setText("Still loading..."), 5000)
    return () => clearTimeout(timer)
  }, [text])

  return (
    <View style={[styles.wrapper, style]}>
      <ActivityIndicator size="large" />
      <Text style={styles.text}>{currentText}</Text>
    </View>
  )
}

[ljharb]

@pke yes, i have commented as much upthread; have you read all the comments?

[pke]

I wanted to emphasise that this rule is important and absolutely doable. I just wanted to start it myself but checked if someone else is already working on it ;) So good luck!

[FengXianGuo]

any one do this?

[ljharb]

Nope, you can tell because the issue’s open.