1

I have a global variable plyViewed in App.js that is set outside of my App component.

let plyViewed = 0;

function App() {

It monitors which move the board game is on. Below the board are some navigation buttons. If you click < I do plyViewed--, if you click I do plyViewed++. You get the picture.

This all worked fine, until I refactored!

I took the navigation buttons, who’s JSX code was all inside the App() function and put it in <MoveButtons /> in MoveButtons.js. So, I thought I could pass down plyViewed as a prop and then update the value in my code in the MoveButton child component. Then I find that props are immutable! Now I am stuck.

My code below gives an example of how I am using that plyViewed code. When someone clicks the navigation buttons, it fires an event that triggers the code to update plyViewed, although now it doesn’t work anymore because it is a prop. The rest of the game data is stored in an object called gameDetails.

I am passing down the plyViewed like this:

<MoveButtons
  plyViewed={plyViewed}
  // etc.
/>

A shortened version of my MoveList component is below.

plyViewed is used in multiple areas throughout my app, like gameDetails. I’ve considered putting it in the gameDetails object, but then I still have the issue of gameDetails being immutable if passed down as a prop. Then if I set plyViewed as a state variable, it becomes asynchronous and therefore unsuitable for use in calculations.

Am I thinking about this all wrong?

export default function MoveButtons(props) {
  return (
    <Grid item xs={6}>
      <Button
        variant="contained"
        color="primary"
        size="large"
        style={{ maxWidth: props.buttonWidth, minWidth: props.buttonWidth }}
        onClick={() => {
          if (props.plyViewed > 0) {
            props.plyViewed--;
            props.board.current.setPosition(props.fenHistory[props.plyViewed]);
            props.setFen(props.fenHistory[props.plyViewed]);
            props.setSelectedIndex(props.plyViewed);
          }
        }}
      >
        <NavigateBeforeIcon />
      </Button>
      <Button
        variant="contained"
        color="primary"
        size="large"
        style={{ maxWidth: props.buttonWidth, minWidth: props.buttonWidth }}
        onClick={() => {
          if (props.plyViewed < props.fenHistory.length - 1) {
            props.plyViewed++;
            props.board.current.setPosition(props.fenHistory[props.plyViewed]);
            props.setFen(props.fenHistory[props.plyViewed]);
            props.setSelectedIndex(props.plyViewed);
          }
        }}
      >
        <NavigateNextIcon />
      </Button>
    </Grid>
  );
}
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
Jon
  • 501
  • 3
  • 18
  • 1
    Hi Jon, try using Redux to manage global state – Ayoub Gharbi Aug 04 '21 at 17:39
  • 1
    Reference for the immutable props: `props.plyViewed++` mutates the [props, which are immutable](https://stackoverflow.com/q/47471131/1218980) in React (even if not enforceable). – Emile Bergeron Aug 04 '21 at 20:16
  • 1
    @AyoubGharbi Yes, I had a bit of a look at Redux, although it seemed quite complex to me and wondered if perhaps Context could be a simpler solution? – Jon Aug 05 '21 at 14:10

3 Answers3

2

you are trying to update the props that are passed down from the higher-level component in your component tree, which is not possible.

You have the option to create a state using React's useState hook and passing down both the value and the dispatcher, but this is not recommended because you would be drilling props down the tree.

You can also pass the onClick events (or parts of them), up to your App component, which is an improvement to the first method but not the best practice in your case.

What you should really be doing is managing your global state using either, React's own Context API, or Redux. I think this could help you out.

Shayan Shojaei
  • 128
  • 1
  • 10
1

While we're missing the full picture, it sounds like plyViewed should be a state and the asynchronous behaviour shouldn't prevent any computation if done properly with React.

It's easy to overlook the fact that the new state value is synchronously computed by ourselves when setting the state. We can just use that same local value to compute anything else and the async behaviour isn't affecting us at all.

    onClick={() => {
      if (props.plyViewed > 0) {

        // New local value computed by ourselves synchronously.
        const updatedPlyViewed = props.plyViewed - 1;

        // Set the state with the new value to reflect changes on the app.
        props.setPlyViewed(updatedPlyViewed);

        // Use the up-to-date local value to compute anything else
        props.board.current.setPosition(props.fenHistory[updatedPlyViewed]);
        props.setFen(props.fenHistory[updatedPlyViewed]);
        props.setSelectedIndex(updatedPlyViewed);
      }
    }}

This is a really simple pattern that should help solve the most basic issues with new state values.

Simple computations

Quick computations can be done in the render phase. The latest state values will always be available at this point. It's unnecessary to sync multiple state values if it can easily be computed from a single value, like the plyViewed here.

const [plyViewed, setPlyViewed] = useState(0);

// No special state or function needed to get the position value.
const position = fenHistory[plyViewed];

Here's an interactive example of how a simple state can be used to compute a lot of different derived information within the render phase.

// Get a hook function
const { useState } = React;

// This component only cares about displaying buttons, the actual logic
// is kept outside, in a parent component.
const MoveButtons = ({ onBack, onNext }) => (
  <div>
    <button type="button" onClick={onBack}>
      Back
    </button>
    <button type="button" onClick={onNext}>
      Next
    </button>
  </div>
);

const App = () => {
  const [fenHistory, setFenHistory] = useState(["a", "b"]);
  const [plyViewed, setPlyViewed] = useState(0);

  const position = fenHistory[plyViewed];

  const onBack = () => setPlyViewed((curr) => Math.max(curr - 1, 0));
  const onNext = () =>
    setPlyViewed((curr) => Math.min(curr + 1, fenHistory.length - 1));

  return (
    <div>
      <p>Ply viewed: {plyViewed}</p>
      <p>Fen position: {position}</p>
      <p>Fen history: {fenHistory.join(", ")}</p>
      <button
        type="button"
        onClick={() =>
          setFenHistory((history) => [...history, `new-${history.length}`])
        }
      >
        Add to fen history
      </button>

      <MoveButtons onBack={onBack} onNext={onNext} />
    </div>
  );
};

// Render it
ReactDOM.render(<App />, document.getElementById("app"));
button {
  margin-right: 5px;
}
<div id="app"></div>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.1/umd/react-dom.production.min.js"></script>

Expensive computations

If the computation takes a considerable amount of time to complete, and that doing it each render cycle is noticeably slowing down the rendering, there are some optimizations we could do.

useMemo will only recompute the memoized value when one of the dependencies has changed. This optimization helps to avoid expensive calculations on every render.

const [plyViewed, setPlyViewed] = useState(0);

const position = useMemo(() => /* costly computation here */, [plyViewed]);

Complex computations

If the computation has a lot of dependencies, we could use useReducer to manage a state object.

Note that the following example isn't justifying the use of useReducer and it's only used as an example of the implementation.

const initialState = {
  plyViewed: 0,
  fenHistory: ["a", "b"],
  positionValue: "a",
};

function reducer(state, action) {
  const { plyViewed, fenHistory } = state;
  switch (action.type) {
    case "back":
      if (fenHistory.length <= 0) return state;
      const newIndex = plyViewed - 1;
      return {
        ...state,
        plyViewed: newIndex,
        positionValue: fenHistory[newIndex],
      };
    case "next":
      if (fenHistory.length - 1 > plyViewed) return state;
      const newIndex = plyViewed + 1;
      return {
        ...state,
        plyViewed: newIndex,
        positionValue: fenHistory[newIndex],
      };
    case "add":
      return {
        ...state,
        fenHistory: [...fenHistory, action.value],
      };
    default:
      throw new Error();
  }
}

const App = () => {
  const [{ plyViewed, fenHistory, positionValue }, dispatch] = useReducer(
    reducer,
    initialState
  );

  const onBack = () => dispatch({ type: "back" });
  const onNext = () => dispatch({ type: "next" });
  const onAdd = () => dispatch({ type: "add", value: 'anything' });
  // ...

Async computations

If we need to get the result, for example, from a distant server, then we could use useEffect which will run once when the value changes.

const App = () => {
  const [plyViewed, setPlyViewed] = useState(0);
  const [position, setPosition] = useState(null);

  useEffect(() => {
    fetchPosition(plyViewed).then((newPosition) => setPosition(newPosition));
  }, [plyViewed]);

There are a couple pitfalls with useEffect and asynchronously setting the state.

Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
  • Thank you for your detailed reply. I've been digesting it to get some nuggets. I never even considered setting a local variable based on the prop. In my instance, that would be easy but then I cannot update plyViewed itself because it is a prop. In your Simple Computations example, is it generally advised to keep the logic out of the child component? plyViewed is used in 70 different lines of code in my app. Is that what is meant when you say there are a lot of "dependencies"? – Jon Aug 05 '21 at 14:09
  • @Jon It's generally advised to keep coupling to a minimum, regardless of the framework used. In this case, the buttons component do not need to manage the whole app, the component only cares about rendering buttons, so it should only have to manage navigation related props and logic. Passing functions is a good way to wrap logic in a parent and pass it down to a child without leaking implementation details and ending up with a tight coupling. – Emile Bergeron Aug 05 '21 at 14:29
  • @jon what I meant by "lots of dependencies" is when some state needs a bunch of different data (e.g. `plyViewed`, a `position`, some object, a user, etc) to be computed properly. Since state updates are async and batched by React, sometimes having lots of individual `useState()` can make every other computation out of sync. `useReducer` makes it easy to manage a state object all at once, with different actions, while isolating the logic to a reducer function, which help minimize coupling as well. – Emile Bergeron Aug 05 '21 at 14:34
  • If I keep the logic in the parent component, do I even need state at all for plyViewed? I can't update it because it was in a child component. But if I keep the logic in the parent and pass down a function instead, I presume I can then update plyViewed because although it is passed down as a function, the function logic was defined in the parent. Or did I get that wrong? – Jon Aug 05 '21 at 16:02
  • @Jon you wouldn't need to pass `plyViewed` anymore as a prop, though it should still be a state so that changes to `plyViewed`, through `setPlyViewed(/**/)`, properly trigger a re-render, which would update what's displayed with the latest computations from `plyViewed` latest value. The logic could then be simplified, like in my interactive example. – Emile Bergeron Aug 05 '21 at 16:04
  • I was thinking that because I am passing a function, that function contains plyViewed. But the function runs in the parent, so the scope remains in the parent, if you see what I mean. I am guessing that is how it works. My app is ridiculously complex for my first go at one. It has about 40 js files and I've probably done a ton of things wrong! There are so many value updates in different variables, I figured plyViewed wouldn't make any difference. – Jon Aug 05 '21 at 16:11
  • @Jon You've set yourself a good challenge! Each time you mutate something that's supposed to be immutable, or use a value before it gets actually updated (wrong scope, wrong lifecycle, etc.), it adds a layer of possible issues on top. So fixing `plyViewed` might not be enough to actually help you much. I'd suggest studying React through a couple tutorials that introduce its different features so that you can build an app with a solid foundation. As suggested by others, you could also look at different state management solutions like Redux, the context API, and many others. – Emile Bergeron Aug 05 '21 at 16:16
  • 1
    The plyViewed issue only came up because I started learning about Refactoring, going through the book with the same name. Then these code breakages started happening. I did go through a tutorial on Udemy. Not finished but lots studied. I had a look at Redux, doing a few lessons, but I'm hoping perhaps context API might be a simpler solution for me when I eventually go down that path. But thanks for your help, it has been great. I will give it a go! – Jon Aug 05 '21 at 16:20
-1

Then if I set plyViewed as a state variable, it becomes asynchronous and therefore unsuitable for use in calculations.

I think this is incorrect, you just need to start using useEffect as well:

function App() {
    const [plyViewed, setPlyViewed] = useState(0);
    <MoveButtons
        plyViewed={plyViewed}
        setPlyViewed={setPlyViewed}
    />
}

export default function MoveButtons(props) {
    useEffect(() => {
        props.board.current.setPosition(props.fenHistory[props.plyViewed]);
        props.setFen(props.fenHistory[props.plyViewed]);
        props.setSelectedIndex(props.plyViewed);
    }, [props.plyViewed, props.fenHistory]);
    return (
    <Grid item xs={6}>
      <Button
        variant="contained"
        color="primary"
        size="large"
        style={{ maxWidth: props.buttonWidth, minWidth: props.buttonWidth }}
        onClick={() => {
          if (props.plyViewed > 0) {
            props.plyViewed--;
          }
        }}
      >
        <NavigateBeforeIcon />
      </Button>
      <Button
        variant="contained"
        color="primary"
        size="large"
        style={{ maxWidth: props.buttonWidth, minWidth: props.buttonWidth }}
        onClick={() => {
        if (props.plyViewed < props.fenHistory.length - 1) {
          props.plyViewed++;
        }}
      >
        <NavigateNextIcon />
      </Button>
    </Grid>
  );
dave
  • 62,300
  • 5
  • 72
  • 93
  • plyViewed is used all over my App though, in other child components, helper libraries etc. For example, in a gameAnalyser.js file, I have plyViewed called like this: ` gameDetails = checkUncastledKing(gameDetails, fen, plyViewed);` Would I then need to do useEffect stuff in that called function too? – Jon Aug 04 '21 at 18:20
  • probably not - if you're already passing down `plyViewed` as a prop, it should continue to work, you really just need to pass down setPlyViewed where you update, but in this component you were trying to update and then use the updated value before the next render. You could have just passed down `setPlyView(newVal); props.setFen(newVal)` but when updating and then using new value I find useEffect to be much cleaner – dave Aug 04 '21 at 18:59