Nit: please keep import lines separate from type lines (add a newline here). Hoping this isn't Prettier's fault, but I suspect it's not because I'm guessing the Flow types get stripped before Prettier sees them. I'm guessing Prettier doesn't make this worse but it probably can't fix it either

It makes no sense that you put setModal in one of these dep lists but not the other.

High-level feedback: try to be more rigorous in how you think about dep lists... they're one of the things you really have to understand 100% to be able to write good React hook code.

More concrete feedback: useState setters never change, so you can treat them just like constants and leave them out of dep lists. That's why eslint-plugin-react-hooks doesn't complain when they're left off of dep lists.

Based on my read of the types here, it seems like you can just pass setModal in directly. Did you do it this way on purpose? I guess an argument can be made that it's more readable this way

handleClick is a bad name here... setModal can be called from contexts that have nothing to do with a click being handled. Can you come up with a better name? Even something generic like setModalAPI or contextSetModal would be better, I think

By passing the object directly in like this, you make both of the React.useCallback calls above completely useless.

In other words: if you had written the exact same code without the React.useCallbacks, the performance would be no worse (in fact, exactly the same). That's because the React.useCallbacks you've implemented fail to stop any rerenders, since they're wrapped in an object that is created anew on every render.

The whole point of React.useMemo, React.useCallback, and dep lists is to reduce unnecessary render cycles. The thinking goes: by memoizing props passed to child components, you make it possible for the child components (if they implement React.PureComponent or use React.memo†) to avoid rerendering whenever the parent rerenders.

This is generally good to keep in mind anywhere, but particularly important when writing a Provider component. By failing to memoize the prop here, you force the children of ModalProvider to have to rerender whenever either ModalProvider rerenders, or an ancestor of ModalProvider rerenders.‡

As it turns out, the latter (ancestor of ModalProvider rerendering) never happens because all of its ancestors are pure components. And as for ModalProvider rerendering, this only happens when setModal or clearModal are called.

By wrapping this object instantiation in a React.useMemo, you may actually achieve a perf benefit when setModal and clearModal are called. In that situation we might avoid rerendering HotRoot. To understand if a perf benefit exists, you would have to understand how react-hot-loader works in prod (this was pretty easy for me) and how react-router works (I didn't manage to figure this out in 10min so gave up).

Given this is a Provider component near the root of the component hierarchy, it's probably safest just to wrap this object instantiation in a React.useMemo.

Try to keep this sort of thing in mind going forward. A good rule of thumb is to be suspicious whenever you see {{ }}.

† Note that this refers to React.memo, not React.useMemo. They are in fact different.
‡ If you're curious to learn more, I wrote a note about this while reviewing this diff.

I'm concerned that you're wrapping HotRoot here. react-hot-loader tends to have a lot of opinions about this stuff, and the docs indicate you should be wrapping the root component of your app.

Instead of putting ModalProvider here, I think we should probably put it in the App component (the one inside app.react.js).

(I vaguely recall there was some good reason react-redux's Provider has to go even higher than react-hot-loader, but I don't recall exactly what that reason was. Don't think we should change that.)

Yeah, totally okay

I think I never got a response to this question:

Based on my read of the types here, it seems like you can just pass setModal in directly. Did you do it this way on purpose? I guess an argument can be made that it's more readable this way

What if you never declared contextSetModal, and just passed in setModal itself directly. Would that work?

Nit: there's a shorthand for this

This seems like an alias for setModal, not clear why it needs to exist

We can use a shorthand to avoid the return keyword