Skip to content

Fix React Compiler compatibility.#1690

Open
phryneas wants to merge 13 commits into
downshift-js:masterfrom
phryneas:pr/react-compiler-compat
Open

Fix React Compiler compatibility.#1690
phryneas wants to merge 13 commits into
downshift-js:masterfrom
phryneas:pr/react-compiler-compat

Conversation

@phryneas

@phryneas phryneas commented Jun 3, 2026

Copy link
Copy Markdown

Pull Request

What

Fix React Compiler compatibility.

Why

Currently, the getter functions are impure and stable. That leads to situations where the React compiler will never re-execute them, keeping state like value at the initial value for the whole lifetime of components.

See #1646

This violated the react-hooks/refs lint rule: https://react.dev/reference/eslint-plugin-react-hooks/lints/refs

How

This PR now makes a split:

  • effects and handers read from refs
  • getters read actual state, and their stability mimicks the stability of state.

It also updates eslint-plugin-react-hooks and enables the /refs rule. 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

  • Documentation
  • Tests
  • TypeScript Types
  • Ready to be merged

@phryneas

phryneas commented Jun 3, 2026

Copy link
Copy Markdown
Author

Oh dang, of course I had to work on an outdated branch... Trying to rebase.

@phryneas phryneas force-pushed the pr/react-compiler-compat branch from f7a7519 to 009b7f0 Compare June 3, 2026 14:49
const actionRef = React.useRef<Action<S, A, P>>()
const enhancedReducer = React.useCallback(
(state: S, action: Action<S, A, P>): S => {
actionRef.current = action

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not allowed to write a ref in a reducer, so I had to add the lastAction to state instead.

Comment on lines 55 to 58
const dispatchWithProps = React.useCallback(
(action: A) => dispatch({...action, props: propsRef.current}),
[propsRef],
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/utils/useLatestRef.ts
Comment on lines -5 to -8
// 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@phryneas

phryneas commented Jun 8, 2026

Copy link
Copy Markdown
Author

@silviuaavram I've also added a compiled version of the useCombobox E2E test.
This fails on master, but succeeds on this branch.
Try it manually in the browser, too - typing in the input field doesn't work at all with the master build.

@phryneas

phryneas commented Jun 8, 2026

Copy link
Copy Markdown
Author

Also, note that this PR only fixes the hooks. I don't believe the Downshift component itself can be fixed apart from a full rewrite - which I'm not going to do.

This might need to be documented on top - users of React Compiler would need to use the hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants