Skip to content

Commit 06400ca

Browse files
committed
[compiler] Refactor event handler validation to use type inference
1 parent d42f435 commit 06400ca

14 files changed

+94
-85
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,15 @@ export const EnvironmentConfigSchema = z.object({
670670
* from refs need to be stored in state during mount.
671671
*/
672672
enableAllowSetStateFromRefsInEffects: z.boolean().default(true),
673+
674+
/**
675+
* Enables inference of event handler types for JSX props on built-in DOM elements.
676+
* When enabled, functions passed to event handler props (props starting with "on")
677+
* on primitive JSX tags are inferred to have the BuiltinEventHandlerId type, which
678+
* allows ref access within those functions since DOM event handlers are guaranteed
679+
* by React to only execute in response to events, not during render.
680+
*/
681+
enableInferEventHandlers: z.boolean().default(false),
673682
});
674683

675684
export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;

compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
BuiltInUseTransitionId,
3030
BuiltInWeakMapId,
3131
BuiltInWeakSetId,
32-
BuiltinEffectEventId,
32+
BuiltInEffectEventId,
3333
ReanimatedSharedValueId,
3434
ShapeRegistry,
3535
addFunction,
@@ -863,7 +863,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
863863
returnType: {
864864
kind: 'Function',
865865
return: {kind: 'Poly'},
866-
shapeId: BuiltinEffectEventId,
866+
shapeId: BuiltInEffectEventId,
867867
isConstructor: false,
868868
},
869869
calleeEffect: Effect.Read,

compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,8 +403,9 @@ export const BuiltInStartTransitionId = 'BuiltInStartTransition';
403403
export const BuiltInFireId = 'BuiltInFire';
404404
export const BuiltInFireFunctionId = 'BuiltInFireFunction';
405405
export const BuiltInUseEffectEventId = 'BuiltInUseEffectEvent';
406-
export const BuiltinEffectEventId = 'BuiltInEffectEventFunction';
406+
export const BuiltInEffectEventId = 'BuiltInEffectEventFunction';
407407
export const BuiltInAutodepsId = 'BuiltInAutoDepsId';
408+
export const BuiltInEventHandlerId = 'BuiltInEventHandlerId';
408409

409410
// See getReanimatedModuleType() in Globals.ts — this is part of supporting Reanimated's ref-like types
410411
export const ReanimatedSharedValueId = 'ReanimatedSharedValueId';
@@ -1243,7 +1244,20 @@ addFunction(
12431244
calleeEffect: Effect.ConditionallyMutate,
12441245
returnValueKind: ValueKind.Mutable,
12451246
},
1246-
BuiltinEffectEventId,
1247+
BuiltInEffectEventId,
1248+
);
1249+
1250+
addFunction(
1251+
BUILTIN_SHAPES,
1252+
[],
1253+
{
1254+
positionalParams: [],
1255+
restParam: Effect.ConditionallyMutate,
1256+
returnType: {kind: 'Poly'},
1257+
calleeEffect: Effect.ConditionallyMutate,
1258+
returnValueKind: ValueKind.Mutable,
1259+
},
1260+
BuiltInEventHandlerId,
12471261
);
12481262

12491263
/**

compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
} from '../HIR/HIR';
2626
import {
2727
BuiltInArrayId,
28+
BuiltInEventHandlerId,
2829
BuiltInFunctionId,
2930
BuiltInJsxId,
3031
BuiltInMixedReadonlyId,
@@ -471,6 +472,32 @@ function* generateInstructionTypes(
471472
}
472473
}
473474
}
475+
if (env.config.enableInferEventHandlers) {
476+
if (value.kind === 'JsxExpression' && value.tag.kind === 'BuiltinTag') {
477+
/*
478+
* Infer event handler types for built-in DOM elements.
479+
* Props starting with "on" (e.g., onClick, onSubmit) on primitive tags
480+
* are inferred as event handlers. This allows functions with ref access
481+
* to be passed to these props, since DOM event handlers are guaranteed
482+
* by React to only execute in response to events, never during render.
483+
*/
484+
for (const prop of value.props) {
485+
if (
486+
prop.kind === 'JsxAttribute' &&
487+
prop.name.startsWith('on') &&
488+
prop.name.length > 2 &&
489+
prop.name[2] === prop.name[2].toUpperCase()
490+
) {
491+
yield equation(prop.place.identifier.type, {
492+
kind: 'Function',
493+
shapeId: BuiltInEventHandlerId,
494+
return: makeType(),
495+
isConstructor: false,
496+
});
497+
}
498+
}
499+
}
500+
}
474501
yield equation(left, {kind: 'Object', shapeId: BuiltInJsxId});
475502
break;
476503
}

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts

Lines changed: 14 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@ import {
1414
BlockId,
1515
HIRFunction,
1616
IdentifierId,
17+
Identifier,
1718
Place,
1819
SourceLocation,
1920
getHookKindForType,
2021
isRefValueType,
2122
isUseRefType,
2223
} from '../HIR';
24+
import {BuiltInEventHandlerId} from '../HIR/ObjectShape';
2325
import {
2426
eachInstructionOperand,
2527
eachInstructionValueOperand,
@@ -183,6 +185,11 @@ function refTypeOfType(place: Place): RefAccessType {
183185
}
184186
}
185187

188+
function isEventHandlerType(identifier: Identifier): boolean {
189+
const type = identifier.type;
190+
return type.kind === 'Function' && type.shapeId === BuiltInEventHandlerId;
191+
}
192+
186193
function tyEqual(a: RefAccessType, b: RefAccessType): boolean {
187194
if (a.kind !== b.kind) {
188195
return false;
@@ -354,7 +361,6 @@ function validateNoRefAccessInRenderImpl(
354361
}
355362

356363
const interpolatedAsJsx = new Set<IdentifierId>();
357-
const usedAsEventHandlerProp = new Set<IdentifierId>();
358364
for (const block of fn.body.blocks.values()) {
359365
for (const instr of block.instructions) {
360366
const {value} = instr;
@@ -364,27 +370,6 @@ function validateNoRefAccessInRenderImpl(
364370
interpolatedAsJsx.add(child.identifier.id);
365371
}
366372
}
367-
if (value.kind === 'JsxExpression') {
368-
/*
369-
* Track identifiers used as event handler props on built-in DOM elements.
370-
* We only allow this optimization for native DOM elements because their
371-
* event handlers are guaranteed by React to only execute in response to
372-
* events, never during render. Custom components could technically call
373-
* their onFoo props during render, which would violate ref access rules.
374-
*/
375-
if (value.tag.kind === 'BuiltinTag') {
376-
for (const prop of value.props) {
377-
if (
378-
prop.kind === 'JsxAttribute' &&
379-
prop.name.startsWith('on') &&
380-
prop.name.length > 2 &&
381-
prop.name[2] === prop.name[2].toUpperCase()
382-
) {
383-
usedAsEventHandlerProp.add(prop.place.identifier.id);
384-
}
385-
}
386-
}
387-
}
388373
}
389374
}
390375
}
@@ -541,8 +526,8 @@ function validateNoRefAccessInRenderImpl(
541526
*/
542527
if (!didError) {
543528
const isRefLValue = isUseRefType(instr.lvalue.identifier);
544-
const isUsedAsEventHandler = usedAsEventHandlerProp.has(
545-
instr.lvalue.identifier.id,
529+
const isEventHandlerLValue = isEventHandlerType(
530+
instr.lvalue.identifier,
546531
);
547532
for (const operand of eachInstructionValueOperand(instr.value)) {
548533
/**
@@ -551,29 +536,16 @@ function validateNoRefAccessInRenderImpl(
551536
*/
552537
if (
553538
isRefLValue ||
539+
isEventHandlerLValue ||
554540
(hookKind != null &&
555541
hookKind !== 'useState' &&
556542
hookKind !== 'useReducer')
557543
) {
558544
/**
559-
* Special cases:
560-
*
561-
* 1. the lvalue is a ref
562-
* In general passing a ref to a function may access that ref
563-
* value during render, so we disallow it.
564-
*
565-
* The main exception is the "mergeRefs" pattern, ie a function
566-
* that accepts multiple refs as arguments (or an array of refs)
567-
* and returns a new, aggregated ref. If the lvalue is a ref,
568-
* we assume that the user is doing this pattern and allow passing
569-
* refs.
570-
*
571-
* Eg `const mergedRef = mergeRefs(ref1, ref2)`
572-
*
573-
* 2. calling hooks
574-
*
575-
* Hooks are independently checked to ensure they don't access refs
576-
* during render.
545+
* Allow passing refs or ref-accessing functions when:
546+
* 1. lvalue is a ref (mergeRefs pattern: `mergeRefs(ref1, ref2)`)
547+
* 2. lvalue is an event handler (DOM events execute outside render)
548+
* 3. calling hooks (independently validated for ref safety)
577549
*/
578550
validateNoDirectRefValueAccess(errors, operand, env);
579551
} else if (interpolatedAsJsx.has(instr.lvalue.identifier.id)) {
@@ -585,25 +557,6 @@ function validateNoRefAccessInRenderImpl(
585557
* render function which attempts to obey the rules.
586558
*/
587559
validateNoRefValueAccess(errors, env, operand);
588-
} else if (isUsedAsEventHandler) {
589-
/**
590-
* Special case: the lvalue is used as an event handler prop
591-
* on a built-in DOM element
592-
*
593-
* For example `<form onSubmit={handleSubmit(onSubmit)}>`. Here
594-
* handleSubmit is wrapping onSubmit to create an event handler.
595-
* Functions passed to event handler wrappers can safely access
596-
* refs because built-in DOM event handlers are guaranteed by React
597-
* to only execute in response to actual events, never during render.
598-
*
599-
* We only allow this for built-in DOM elements (not custom components)
600-
* because custom components could technically call their onFoo props
601-
* during render, which would violate ref access rules.
602-
*
603-
* We allow passing functions with ref access to these wrappers,
604-
* but still validate that direct ref values aren't passed.
605-
*/
606-
validateNoDirectRefValueAccess(errors, operand, env);
607560
} else {
608561
validateNoRefPassedToFunction(
609562
errors,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
## Input
33

44
```javascript
5+
// @enableInferEventHandlers
56
// @validateRefAccessDuringRender
67
import {useRef} from 'react';
78

@@ -56,7 +57,8 @@ export const FIXTURE_ENTRYPOINT = {
5657
## Code
5758
5859
```javascript
59-
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
60+
import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers
61+
// @validateRefAccessDuringRender
6062
import { useRef } from "react";
6163

6264
// Simulates react-hook-form's handleSubmit

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @enableInferEventHandlers
12
// @validateRefAccessDuringRender
23
import {useRef} from 'react';
34

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
## Input
33

44
```javascript
5+
// @enableInferEventHandlers
56
// @validateRefAccessDuringRender
67
import {useRef} from 'react';
78

@@ -44,7 +45,8 @@ export const FIXTURE_ENTRYPOINT = {
4445
## Code
4546

4647
```javascript
47-
import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender
48+
import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers
49+
// @validateRefAccessDuringRender
4850
import { useRef } from "react";
4951

5052
// Simulates react-hook-form's handleSubmit or similar event handler wrappers

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @enableInferEventHandlers
12
// @validateRefAccessDuringRender
23
import {useRef} from 'react';
34

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
## Input
33

44
```javascript
5+
// @enableInferEventHandlers
56
// @validateRefAccessDuringRender
67
import {useRef} from 'react';
78

@@ -56,14 +57,14 @@ Error: Cannot access refs during render
5657
5758
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
5859
59-
error.ref-value-in-custom-component-event-handler-wrapper.ts:31:41
60-
29 | <>
61-
30 | <input ref={ref} />
62-
> 31 | <CustomForm onSubmit={handleSubmit(onSubmit)}>
60+
error.ref-value-in-custom-component-event-handler-wrapper.ts:32:41
61+
30 | <>
62+
31 | <input ref={ref} />
63+
> 32 | <CustomForm onSubmit={handleSubmit(onSubmit)}>
6364
| ^^^^^^^^ Passing a ref to a function may read its value during render
64-
32 | <button type="submit">Submit</button>
65-
33 | </CustomForm>
66-
34 | </>
65+
33 | <button type="submit">Submit</button>
66+
34 | </CustomForm>
67+
35 | </>
6768
```
6869
6970

0 commit comments

Comments
 (0)