invoke-listeners-in-render

[dai-shi]

When I implemented #26, I assumed it's safer to invoke listeners in useLayoutEffect.
However, this leads to "de-optimize to sync mode" [1]. And, it's actually slow [2].

This changes to invoke listeners in render. There might be some cases that prevent from properly working. I would like to know such cases. Eventually someone would try it an file an issue. Will see.

[markerikson]

This would seem to be problematic specifically because the point of Concurrent Mode is that existing in-progress trees can be thrown away.

[dai-shi]

Yeah, the tree can be thrown away, but does that mean the local state (in this case the forceUpdate counts) is thrown away, which prevents re-renders?
If so, we should delete the line ref.current.state = nextState; so that the next callback triggers re-render.
FWIW, I tried this approach in the tool, and it seems good so far. I would like to create a failing scenario, if there's such a problem.

@markerikson how can we convince this approach works/fails? any ideas?

[dai-shi]

Okay, I was able to at last reproduce the thrown-away behavior.
You are absolutely right, @markerikson .
I could see inconsistency with the current modification.

And, by removing ref.current.state = nextState; it works without inconsistency after the thrown-away behavior.
It is still an experiment with only one tiny counter app, but I got a little confident with this approach now.

[dai-shi]

So, here's a big guess. It's natural to assume that we don't need batchedUpdates in render.

https://github.com/facebook/react/blob/b4178af81b02dfa0e898670ce564c4649fd8947f/packages/react-reconciler/src/ReactFiberWorkLoop.js#L597
Not quite sure how BatchedContext is used, but anyway you couldn't start render if it's already in render.

---

Just made a small example.
https://codesandbox.io/s/old-wood-n34dw
With it, I'd assume no batchedUpdates is required.

[dai-shi]

Summary:
Although invoking callbacks in render sounds generally a bad idea, but my current understanding is if we carefully implement it, it works fine with the calculateChangedBits = () => 0; technique.
Unless I receive more feedback for various cases, it can't be said to be really ok, but to get feedback I should release it.
With that in mind, I'd merge this PR and encourage people to try it.

This specific change is not tied to state usage tracking, so any react-redux hooks app can be simply tried by replacing the library.

[dai-shi]

https://github.com/facebook/react/blob/ce883a19d845e1faf8d4e1587e7022feda66210a/packages/use-subscription/src/useSubscription.js#L51-L55
This code is invoking setState conditionally in render. Maybe, it's not so bad.