Skip to content

Compiler: NonReactive annotation#1

Draft
srubin wants to merge 11 commits intomainfrom
steve/stable-handler
Draft

Compiler: NonReactive annotation#1
srubin wants to merge 11 commits intomainfrom
steve/stable-handler

Conversation

@srubin
Copy link
Copy Markdown

@srubin srubin commented Feb 6, 2026

Summary

Automatic stable-identity codegen for handler-style functions in components, gated behind new feature flags. The goal is to reduce the amount of manual memoization (useCallback, custom annotations) users have to write for the common case of "inline function that's only passed as a JSX prop."

There are two layers:

1. NonReactive<T> annotation + two-slot codegen (already in earlier commits on this branch)

Users can type a local handler as NonReactive<T> (or wrap it with nonReactive(...)). The compiler then emits a two-slot cache for that function:

t1 = () => doThing(value);           // latest closure, updated every render
$[0] = t1;
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
  t1 = (...args) => $[0](...args);   // stable wrapper, created once
  $[1] = t1;
}

The wrapper has stable identity for downstream consumers but always delegates to the current closure, so captured values stay fresh.

This powers: excluding the handler from reactive deps (InferReactivePlaces, ValidateExhaustiveDependencies, InferEffectDependencies), and enabling ref-access through it.

2. Usage-based inference (new in this PR's latest commit)

Gated behind enableInferNonReactiveHandlers. A new pass inferNonReactiveHandlers runs after inferTypes and, for each FunctionExpression in the component, checks whether every transitive use flows only into safe sinks:

  • Safe: JSX attribute, useEffect / useLayoutEffect / useInsertionEffect / useEffectEvent argument, aliasing transfers (LoadLocal / StoreLocal / phi).
  • Bails out on: direct call, argument to a non-effect hook (useMemo, useCallback, useState, custom hooks), stored in an object or array (escape), returned from the component, flowed through a context variable, or an explicit Reactive<T> opt-out annotation.

If every use is safe, the function's identifier types are mutated to BuiltInNonReactive and the existing markNonReactiveScopes pass drives the two-slot codegen unchanged. Also handles inline arrows in JSX (no intermediate const required) via a small extension to MarkNonReactiveScopes.

Escape hatches

  • Reactive<T> annotation on a local to opt out of the inference (preferred — more grokkable at the call site)
  • Explicit useCallback still works
  • Local NonReactive<T> annotation and nonReactive() wrapper remain as forceful opt-ins for cases the inference bails out on

The prop-level NonReactive<T> annotation (previously on component parameters) was removed in this PR — the use cases (cross-module stability guarantees, suppressing Child's exhaustive-deps warnings, ref access through a passed-in prop) were deemed not worth the complexity right now.

Feature flags

  • enableNonReactiveAnnotation — existing gate for the annotation + two-slot codegen.
  • enableUseTypeAnnotations — prerequisite for reading NonReactive<T> / Reactive<T> off variable declarations.
  • enableInferNonReactiveHandlers — new flag for the usage-based inference. Requires enableNonReactiveAnnotation.

All default to false — no behavior change for existing users.

How did you test this change?

  • Full snapshot suite passes: 1850 / 1850 (yarn snap).
  • yarn tsc --noEmit clean.
  • New fixtures under compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/non-reactive/:
    • infer-non-reactive-inline-handler — baseline: const h = () => …; <button onClick={h}/> → two-slot codegen
    • infer-non-reactive-inline-jsx — inline arrow directly in JSX attribute → two-slot
    • infer-non-reactive-aliased — handler relayed through a second local
    • infer-non-reactive-reactive-optoutReactive<T> keeps the value-cached codegen
    • infer-non-reactive-bailout-local-call — calling the handler in render bails out
    • infer-non-reactive-bailout-usememo — passing to useMemo bails out
    • infer-non-reactive-bailout-object-escape — storing in an object bails out
    • non-reactive-prop-propagation — cross-component: Parent's handler gets stabilized, Child caches its JSX on a stable identity so it never invalidates

Prompt: Looking at @compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/stable-handler/stable-handler-prop-basic.expect.md -- I see
   two potential problems. It looks to me like the onClick that is being passed to <button> will be re-created whenever `value` changes. It also looks
  like `t1` is only capturing the value of `onSubmit` when `value` changes. What we want instead is for `onClick` to never update, and to always
  capture the latest values of `value` and `onSubmit`.
@srubin srubin self-assigned this Feb 6, 2026
srubin and others added 8 commits February 6, 2026 16:58
Adds enableInferNonReactiveHandlers: a new pass in TypeInference that
scans each FunctionExpression's uses within a component and, if every
use is safe (JSX attribute or effect-hook argument), marks the
function's identifiers as NonReactive. MarkNonReactiveScopes then drives
the existing two-slot stable-identity codegen. Removes the need to
manually annotate handlers with NonReactive<T> or wrap them in
nonReactive() in the common case.

- Bails out on direct calls, non-effect hook args (useMemo etc), object
  or array escape, return-from-component, and context flow.
- Adds Reactive<T> as an opt-out annotation for cases where the
  inference would mark a handler but the user wants reactive identity.
- Extends MarkNonReactiveScopes to recognize FunctionExpressions whose
  own lvalue is NonReactive-typed, so inline arrows in JSX attributes
  are also stabilized without an intermediate const assignment.
- Drops the now-unused prop-level NonReactive<T> plumbing
  (propsTypeAnnotations on HIRFunction, extractPropsTypeAnnotations,
  InferTypes Destructure handling). Local NonReactive<T> annotations
  and the nonReactive() wrapper remain as explicit overrides.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflicts with upstream's feature-flag cleanup (facebook#35825):
- Keep local enableUseTypeAnnotations, enableNonReactiveAnnotation, and
  new enableInferNonReactiveHandlers flags in Environment.ts — our
  NonReactive work still needs them.
- Accept upstream deletion of InferEffectDependencies.ts and the
  BuiltInAutodepsId / BuiltInEventHandlerId shapes (the autodeps/Fire
  feature was abandoned upstream).
- Keep BuiltInNonReactiveId registration in ObjectShape.ts and add a
  matching BuiltInReactiveId registration for the Reactive<T> opt-out.
- Drop dead isEventHandlerType helper from ValidateNoRefAccessInRender.
- Re-add lowerType import in InferTypes.ts since our StoreLocal branch
  needs it.

yarn snap: 1731 / 1731 passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…facebook#36322)

## Summary

Follow-up to facebook#36148 (which added credentialless as a recognized boolean
attribute for iframes). Adds credentialless to possibleStandardNames so
React's dev warning can suggest the correct casing when users write it
as Credentialless (or another incorrect case). Includes an SSR test
asserting the "Did you mean credentialless?" warning fires.

 ## Test plan

- yarn test ReactDOMComponent passes, including the new should warn
about incorrect casing on the credentialless property (ssr) case
…re options per DOM spec (facebook#36047)

## Summary

`FragmentInstance.addEventListener` and `removeEventListener` fail to
cross-match listeners when the `capture` option is passed as a
**boolean** in one call and an **options object** in the other. This
violates the [DOM Living
Standard](https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener),
which states that `addEventListener(type, fn, true)` and
`addEventListener(type, fn, {capture: true})` are identical.

### Root Cause

In `ReactFiberConfigDOM.js`, the `normalizeListenerOptions` function
generates a listener key string for deduplication. The boolean branch
generates a **different format** than the object branch:

```js
// Boolean branch (old) — produces "c=1"
return `c=${opts ? '1' : '0'}`;

// Object branch — produces "c=1&o=0&p=0"
return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`;
```

Because the keys differ, `indexOfEventListener` cannot match them — so
`removeEventListener('click', fn, {capture: true})` silently fails to
remove a listener registered with `addEventListener('click', fn, true)`,
and vice versa. This causes a **memory leak and event listener
accumulation** on all Fragment child DOM nodes.

### Fix

Normalize the boolean branch to produce the same full key format:

```js
// Boolean branch (fixed) — now produces "c=1&o=0&p=0" (matches object branch)
return `c=${opts ? '1' : '0'}&o=0&p=0`;
```

This makes both forms produce an identical key, matching the DOM spec
behavior.

### When Was This Introduced

This bug has been present since `FragmentInstance` event listener
tracking was first added. It became reachable in production as of
[facebook#36026](facebook#36026) which enabled
`enableFragmentRefs` + `enableFragmentRefsInstanceHandles` across all
builds (merged 3 days ago).

### Tests

Added two new regression tests to `ReactDOMFragmentRefs-test.js`:

1. `removes a capture listener registered with boolean when removed with
options object`
2. `removes a capture listener registered with options object when
removed with boolean`

Both tests were failing before this fix and pass after.

## How did you test this change?

Added two new automated tests covering both cross-form removal
directions. Existing tests continue to pass.

## Changelog

### React DOM
- **Fixed** `FragmentInstance.removeEventListener()` not removing
capture-phase listeners when the `capture` option form (boolean vs
options object) differs between `add` and `remove` calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants