Multiple re-rendering by useHotkey

[HZ-labs]

Inside List components (Row) have empty hotkeys. On Scrolling list - rendering Row causes App with use useHotkey to re-render.

Environment

• Package version(s): @blueprintjs/core v4.0.0-rc.0, react-virtualized v9.22.3
• Operating System: Mac os 12.2.1
• Browser name and version: Chrome 99.0.4844.51

Code Sandbox

Code sandbox

Steps to reproduce

Loom - https://www.loom.com/share/f6ad7a40cdd94bc08a2f36f7ac23a5db

Actual behavior

Component is re-rendering when scrolling List

Expected behavior

Component doesn't re-rendering when scrolling List

[stephen]

I had a similar issue and looked at this briefly and see two problems:

1. HotkeysProvider passes an unmemoized array to the context, so the value is constantly changing: https://github.com/palantir/blueprint/blob/364ec0937526bd819947ca17a0537f8a0c52f7fc/packages/core/src/context/hotkeys/hotkeysProvider.tsx#L126

This value needs be wrapped in a useMemo.

2. Even if that value is useMemod, the passed state is constantly changing because the hotkeys are changing (i.e. in the react-virtualized case, the list components are being mounted/unmounted and useHotkeys updates the hotkeys state) constantly.

However, useHotkeys doesn't actually need to see the hotkeys value: https://github.com/palantir/blueprint/blob/364ec0937526bd819947ca17a0537f8a0c52f7fc/packages/core/src/hooks/hotkeys/useHotkeys.ts#L78-L82

The only usage is to warn for incorrect usage. We could fix this by only passing the used hasProvider value as the context state to prevent unnecessary re-renders.

I've tested these two changes and verify the prevent additional re-renders from happening. Since HotkeysContext is unfortunately exported as part of @blueprint/core, it's possible that current library users rely on the current type for the context. To avoid a breaking change, perhaps we could introduce a new HotkeysDialogProvider that does not pass the hotkey configuration?

[stephen]

As a hack, I wrote a drop-in replacement that implements the suggested changes: https://gist.github.com/stephen/873773683fc15d1fb290b338a6c59d38

The main issue is that direct importers of HotkeysContext will not see the hotkeys array, but I don't do that in my app.

[pbower]

As a Blueprint user, I can confirm that this HotKeys provider in the main package caused infinite re-renders in a standard setting, that was immediately resolved through using Stephen's version above.

https://blueprintjs.com/docs/#core/context/hotkeys-provider.usage

[adidahiya]

@stephen I took a quick look at this and attempted to apply your gist to the Blueprint code base. I started a draft PR here: https://github.com/palantir/blueprint/pull/6643 - feel free to branch off from there

The code had some issues violating the rules of hooks and passing type checking (we don't want to cast to any to get a reduced subset of state). If you can figure out a way to redesign the hotkeys provider in a way that passes lint & type checks, I'm open to a PR to fix this issue. I don't mind introducing a HotkeysProvider2 if the solution involves breaking changes.