Fix React Compiler compatibility.#1690
Conversation
|
Oh dang, of course I had to work on an outdated branch... Trying to rebase. |
f7a7519 to
009b7f0
Compare
| const actionRef = React.useRef<Action<S, A, P>>() | ||
| const enhancedReducer = React.useCallback( | ||
| (state: S, action: Action<S, A, P>): S => { | ||
| actionRef.current = action |
There was a problem hiding this comment.
It's not allowed to write a ref in a reducer, so I had to add the lastAction to state instead.
| const dispatchWithProps = React.useCallback( | ||
| (action: A) => dispatch({...action, props: propsRef.current}), | ||
| [propsRef], | ||
| ) |
There was a problem hiding this comment.
You could theoretically skip all of this, and just access props directly in the reducer (and save it to the side there, the same way I did with lastAction), while removing the useCallback on the reducer.
The logic behind this is that calling dispatch will immediately rerender the reducer-owning component, and since the parents and other inputs won't change, props will always be the newest. See [Why useReducer Is the Cheat Mode of Hooks](https://overreacted.io/a-complete-guide-to-useeffect/#why-usereducer-is-the-cheat-mode-of-hooks).
That said, you seem to recently have touched this a lot, so I tried to leave it alone.
| // technically this is not "concurrent mode safe" because we're manipulating | ||
| // the value during render (so it's not idempotent). However, the places this | ||
| // hook is used is to support memoizing callbacks which will be called | ||
| // *during* render, so we need the latest values *during* render. |
There was a problem hiding this comment.
This access during render was pretty much the core problem leading to this PR, so I decided to delete this central hook and inline it where it's still needed with an useEffect. Happy to change it to a useLayoutEffect tho.
Pull Request
What
Fix React Compiler compatibility.
Why
Currently, the
getterfunctions are impure and stable. That leads to situations where the React compiler will never re-execute them, keeping state likevalueat the initial value for the whole lifetime of components.See #1646
This violated the
react-hooks/refslint rule: https://react.dev/reference/eslint-plugin-react-hooks/lints/refsHow
This PR now makes a split:
It also updates
eslint-plugin-react-hooksand enables the/refsrule. There are more rules that should be enabled, but this is the most important one for what this library is doing here.Changes
I tried to do small commits here, so please look at the list of commits.
Note that this changes the signature of a few helpers. This might warrant a major release.
Checklist