robot-hooks: Infinite loop if machine is created anew each render

[dceddia]

Hey, thanks for Robot! It's great. We're using it on a project to manage some tricky online/offline sync workflows and it has simplified everything quite a bit.

I'm actually in the middle of writing a tutorial around it, and hit upon what I think is a bug, but I'll describe what's happening.

My machine implements a "confirm to delete" flow, and it has an invoke state to call the function. Initially I had this deleteFn declared inside the file, hardcoded. Worked fine, but I wanted to parameterize it, so I wrapped the machine in a function...

const confirmationFlow = (deleteFn) =>
  createMachine({
    initial: state(transition("begin", "prompting")),
    prompting: state(
      transition("confirm", "loading"),
      transition("cancel", "initial")
    ),
    loading: invoke(
      deleteFn,
      transition("done", "initial"),
      transition(
        "error",
        "prompting",
        reduce((context, event) => {
          return {
            ...context,
            error: event.error
          };
        })
      )
    )
  });

And then I passed the function to useMachine:

const deleteThings = async () => { /* ... */ }

function App() {
  const [current, send] = useMachine(confirmationFlow(deleteThings));
  // ...
}

This creates an infinite loop because the useEffect inside useMachine is re-rendering every time the argument changes, which happens on every render.

One workaround is to pull the definition out...

const mac = confirmationFlow(deleteThings)

function App() {
  const [current, send] = useMachine(mac);
  // ...
}

But I think then the machine could be shared between multiple component instances, and can't access anything in the component scope like callbacks.

I can also work around this by passing the deleteFn function along with an event - parameterizing at call time, instead of init time.

Mostly, though, I sorta expected that useMachine would work the same as useState, where it "latches" the first value it sees and ignores future changes. I also tried passing a function to resolve the value (like useMachine(() => confirmationFlow(deleteThings))) but that throws an error.

I think one fix would be to change the behavior so that useMachine doesn't re-render when the passed value changes, but if that's undesirable, maybe supporting the function initializer approach would be a good in-between?

[matthewp]

Sorry for the (many years) late response. The current behavior is intentional, allowing you to write high-order hooks that can react to changes in the underlying machine. As you're probably aware, you can work around your issue by using useMemo like so:

const [machine] = useMemo(() => confirmationFlow(deleteThings));
const [current] = useMachine(machine);

So your suggestion is to make useMachine take an optional first arg of a function and then treat it like a memo in that case.

I guess my feeling here is that if you know about this second signature to useMachine you probably also know that you can use useMemo. In which case it feels like it's not much of enhancement. Curious if you agree or not.