Skip to content

Commit 057cb35

Browse files
committed
[compiler] Infer render helpers for additional validation
We currently assume that any functions passes as props may be event handlers or effect functions, and thus don't check for side effects such as mutating globals. However, if a prop is a function that returns JSX that is a sure sign that it's actually a render helper and not an event handler or effect function. So we now emit a `Render` effect for any prop that is a JSX-returning function, triggering all of our render validation. This required a small fix to InferTypes: we weren't correctly populating the `return` type of function types during unification. I also improved the printing of types so we can see the inferred return types.
1 parent ba4bdb2 commit 057cb35

File tree

5 files changed

+77
-1
lines changed

5 files changed

+77
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,8 @@ export function printType(type: Type): string {
892892
if (type.kind === 'Object' && type.shapeId != null) {
893893
return `:T${type.kind}<${type.shapeId}>`;
894894
} else if (type.kind === 'Function' && type.shapeId != null) {
895-
return `:T${type.kind}<${type.shapeId}>`;
895+
const returnType = printType(type.return);
896+
return `:T${type.kind}<${type.shapeId}>()${returnType !== '' ? returnType : ''}`;
896897
} else {
897898
return `:T${type.kind}`;
898899
}

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
InstructionKind,
2727
InstructionValue,
2828
isArrayType,
29+
isJsxType,
2930
isMapType,
3031
isPrimitiveType,
3132
isRefOrRefValue,
@@ -1841,6 +1842,19 @@ function computeSignatureForInstruction(
18411842
});
18421843
}
18431844
}
1845+
for (const prop of value.props) {
1846+
if (
1847+
prop.kind === 'JsxAttribute' &&
1848+
prop.place.identifier.type.kind === 'Function' &&
1849+
isJsxType(prop.place.identifier.type.return)
1850+
) {
1851+
// Any props which return jsx are assumed to be called during render
1852+
effects.push({
1853+
kind: 'Render',
1854+
place: prop.place,
1855+
});
1856+
}
1857+
}
18441858
}
18451859
break;
18461860
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,15 @@ class Unifier {
777777
return {kind: 'Phi', operands: type.operands.map(o => this.get(o))};
778778
}
779779

780+
if (type.kind === 'Function') {
781+
return {
782+
kind: 'Function',
783+
isConstructor: type.isConstructor,
784+
shapeId: type.shapeId,
785+
return: this.get(type.return),
786+
};
787+
}
788+
780789
return type;
781790
}
782791
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
2+
## Input
3+
4+
```javascript
5+
function Component() {
6+
const renderItem = item => {
7+
// Normally we assume that it's safe to mutate globals in a function passed
8+
// as a prop, because the prop could be used as an event handler or effect.
9+
// But if the function returns JSX we can assume it's a render helper, ie
10+
// called during render, and thus it's unsafe to mutate globals or call
11+
// other impure code.
12+
global.property = true;
13+
return <Item item={item} value={rand} />;
14+
};
15+
return <ItemList renderItem={renderItem} />;
16+
}
17+
18+
```
19+
20+
21+
## Error
22+
23+
```
24+
Found 1 error:
25+
26+
Error: This value cannot be modified
27+
28+
Modifying a variable defined outside a component or hook is not allowed. Consider using an effect.
29+
30+
error.invalid-mutate-global-in-render-helper-prop.ts:8:4
31+
6 | // called during render, and thus it's unsafe to mutate globals or call
32+
7 | // other impure code.
33+
> 8 | global.property = true;
34+
| ^^^^^^ value cannot be modified
35+
9 | return <Item item={item} value={rand} />;
36+
10 | };
37+
11 | return <ItemList renderItem={renderItem} />;
38+
```
39+
40+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
function Component() {
2+
const renderItem = item => {
3+
// Normally we assume that it's safe to mutate globals in a function passed
4+
// as a prop, because the prop could be used as an event handler or effect.
5+
// But if the function returns JSX we can assume it's a render helper, ie
6+
// called during render, and thus it's unsafe to mutate globals or call
7+
// other impure code.
8+
global.property = true;
9+
return <Item item={item} value={rand} />;
10+
};
11+
return <ItemList renderItem={renderItem} />;
12+
}

0 commit comments

Comments
 (0)