Unexpected behavior with useEffect + useState

[NinjaNas]

I have a Next.js canvas drawing app and I am trying to use Keystrokes to control state.

Unless I am not using this correctly, I expect it to work like addEventListener in a useEffect and only run once when I hit the key combo. However, it often runs multiple times and even bypasses the index > 0 condition becoming negative when I control+z. Also, my streamActions function to redraw the updated state is seemingly stale even though the index is changing.

I have tried the react hook, it was working a bit better but was still running into the issue where it runs multiple times and index goes to zero.

Here is a snippet of the code:

const [history, setHistory] = useState<Rough.ActionHistory[]>([]);
const [index, setIndex] = useState<number>(0);

// ....

useEffect(() => {
    const onPressedHandler = (e: KeyComboEvent<KeyboardEvent>) => {
      // undo and redo
      if (!roughRef.current) return;

      switch (e.keyCombo) {
        case "control+z":
          index > 0 && setIndex((i) => i - 1);
          break;
        case "control+y":
          index < history.length && setIndex((i) => i + 1);
          break;
      }
    };

    const onReleasedHandler = (e: KeyComboEvent<KeyboardEvent>) => {
      if (!roughRef.current) return;
      streamActions(roughRef.current);
    };

    bindKeyCombo("control+z", {
      onPressed: onPressedHandler,
      onReleased: onReleasedHandler,
    });
    bindKeyCombo("control+y", {
      onPressed: onPressedHandler,
      onReleased: onReleasedHandler,
    });

    return () => {
      unbindKeyCombo("control+z");
      unbindKeyCombo("control+y");
    };
  }, [index]);

[RobertWHurst]

@NinjaNas It looks like every time the index changes the binding is being destroyed and recreated. This means that the binding state (including the handling of key repeat) is being cleared each time the index value is changed.

The other thing I've noticed is that setIndex is called with a function - updating it's value relative to the current value rather than the value of index (which might be different due to how react schedules renders and effects).

Can I see how you attempted to do this using the react package? Perhaps I can help you with that path instead? I think that might yield a better result for you as it takes some of the complexity around the useEffect and binding callbacks out of the picture.

[NinjaNas]

Thanks for responding. I see why that might be an issue. Perhaps adding a useCallback to save the binding might work?

I think that originally I wasn't using the react package correctly and was just using {bindKeyCombo, unbindKeyCombo} from that import...

I have now attempted to use the hooks {useKey, useKeyCombo} in the react package, however I am getting a ReferenceError: $90yQ7$getGlobalKeystrokes is not defined.

It looks like the function is not getting called correctly in @rwh/react-keystrokes/dist/module.js?

[RobertWHurst]

Yikes. The tests are passing for the package but I suspect you are correct that there is something is wrong in the package. Let me take a quick look.

[NinjaNas]

@RobertWHurst Oh, I looked over my old commits and did find that the hooks did work in 1.0.0-beta.12.

Currently testing it out on 1.0.0-beta.12 but it seems to be acting strangely.

useKeyCombo('control+z') requires the control+z to be held and doesn't work on press down. useKeyCombo('shift+z') doesn't work until shift and z are both held and then shift is released

My code is here: https://github.com/NinjaNas/HarmonyScape/blob/keystrokes/hooks/useCanvas.ts

You can control+f isUndo and isRedo.

[NinjaNas]

Okay, I managed to fix control+z on pressed but it stopped working when held. I just removed history and index from the dependency array. Apparently it was running too often making the index go straight to zero.

However, shift+z is still acting the same way.

Update: I added a timer and control+z is working correctly allowing for repeated actions when pressed but now on pressed is delayed but doesn't go directly to zero.

    let intervalId: NodeJS.Timer;
    if (isUndo) {
      intervalId = setInterval(() => {
        undoRedoHandler(undo);
      }, 100);
    } else if (isRedo) {
      intervalId = setInterval(() => {
        undoRedoHandler(redo);
      }, 100);
    }
    return () => {
      if (intervalId) {
        clearInterval(intervalId);
      }
    };
  }, [isUndo, isRedo, index, history]);

[RobertWHurst]

Gotcha. I'll take a look at your code shortly. Just about to put up a PR for #5 - switching to vite. Turns out the error you were running into was caused by an error in parcel. It failed to correctly name the exported variables in the file. I'm glad you ran into this, it needs to be fixed asap. I've just got the vue package to do next, so I'll finish that up then take a look at your code and follow up with you.

[RobertWHurst]

Ok, I just published a new version, let me know if you run into problems.

As for your code, the next suggestion I have is take your code that handles undo out of the useEffect and into a useCallback. The next thing I would suggest is have a useEffect specifically for when the value of the combo changes - as the only dependency in your use effect deps array. Otherwise you will be checking that bool any time any of the deps change, and you don't have a way to tell if you already have handled the current value of isUndo.

I always recommend to people to have small useEffects with only the dependencies you have to have. In my opinion this is the hardest part about React.

[RobertWHurst]

I believe the library is working as expected so I'm going to close the ticket, but I'm happy to carry on the conversation and help you with your code.

[NinjaNas]

Thanks for the help. I believe I got it working correctly. The solution was to split the functions up into a useCallback and useEffect and create a ref for history and index so the handler will always receive the newest value.

I do want to mention that the useCombo(shift + anyOtherKey) is acting not as expected still.

From earlier:

useKeyCombo('shift+z') doesn't work until shift and z are both held and then shift is released

[RobertWHurst]

Yep, you're absolutely correct. Found the issue, I wasn't properly lowercasing the key value in all cases. Will be pushing a patch version in the next few minutes. Thanks again @NinjaNas.

[RobertWHurst]

Ok should be fixed now. I've just released v1.1.1. Let me know if you have any further issues.