Skip to content

Commit 9df57a0

Browse files
committed
Update on "[compiler] Validate type configs for hooks/non-hooks"
Alternative to #30868. The goal is to ensure that the types coming out of moduleTypeProvider are valid wrt to hook typing. If something is named like a hook, then it must be typed as a hook (or don't type it). [ghstack-poisoned]
1 parent c26ca80 commit 9df57a0

6 files changed

+77
-8
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,8 @@ export class Environment {
738738
this.#globals,
739739
this.#shapes,
740740
moduleConfig,
741+
moduleName,
742+
loc,
741743
);
742744
} else {
743745
moduleType = null;

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

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import {
2828
import {BuiltInType, PolyType} from './Types';
2929
import {TypeConfig} from './TypeSchema';
3030
import {assertExhaustive} from '../Utils/utils';
31+
import {isHookName} from './Environment';
32+
import {CompilerError, SourceLocation} from '..';
3133

3234
/*
3335
* This file exports types and defaults for JavaScript global objects.
@@ -535,6 +537,8 @@ export function installTypeConfig(
535537
globals: GlobalRegistry,
536538
shapes: ShapeRegistry,
537539
typeConfig: TypeConfig,
540+
moduleName: string,
541+
loc: SourceLocation,
538542
): Global {
539543
switch (typeConfig.kind) {
540544
case 'type': {
@@ -567,7 +571,13 @@ export function installTypeConfig(
567571
positionalParams: typeConfig.positionalParams,
568572
restParam: typeConfig.restParam,
569573
calleeEffect: typeConfig.calleeEffect,
570-
returnType: installTypeConfig(globals, shapes, typeConfig.returnType),
574+
returnType: installTypeConfig(
575+
globals,
576+
shapes,
577+
typeConfig.returnType,
578+
moduleName,
579+
loc,
580+
),
571581
returnValueKind: typeConfig.returnValueKind,
572582
noAlias: typeConfig.noAlias === true,
573583
mutableOnlyIfOperandsAreMutable:
@@ -580,7 +590,13 @@ export function installTypeConfig(
580590
positionalParams: typeConfig.positionalParams ?? [],
581591
restParam: typeConfig.restParam ?? Effect.Freeze,
582592
calleeEffect: Effect.Read,
583-
returnType: installTypeConfig(globals, shapes, typeConfig.returnType),
593+
returnType: installTypeConfig(
594+
globals,
595+
shapes,
596+
typeConfig.returnType,
597+
moduleName,
598+
loc,
599+
),
584600
returnValueKind: typeConfig.returnValueKind ?? ValueKind.Frozen,
585601
noAlias: typeConfig.noAlias === true,
586602
});
@@ -589,10 +605,31 @@ export function installTypeConfig(
589605
return addObject(
590606
shapes,
591607
null,
592-
Object.entries(typeConfig.properties ?? {}).map(([key, value]) => [
593-
key,
594-
installTypeConfig(globals, shapes, value),
595-
]),
608+
Object.entries(typeConfig.properties ?? {}).map(([key, value]) => {
609+
const type = installTypeConfig(
610+
globals,
611+
shapes,
612+
value,
613+
moduleName,
614+
loc,
615+
);
616+
const expectHook = isHookName(key);
617+
let isHook = false;
618+
if (type.kind === 'Function' && type.shapeId !== null) {
619+
const functionType = shapes.get(type.shapeId);
620+
if (functionType?.functionType?.hookKind !== null) {
621+
isHook = true;
622+
}
623+
}
624+
if (expectHook !== isHook) {
625+
CompilerError.throwInvalidConfig({
626+
reason: `Invalid type configuration for module`,
627+
description: `Expected type for object property '${key}' from module '${moduleName}' ${expectHook ? 'to be a hook' : 'not to be a hook'} based on the property name`,
628+
loc,
629+
});
630+
}
631+
return [key, type];
632+
}),
596633
);
597634
}
598635
default: {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
2+
## Input
3+
4+
```javascript
5+
import ReactCompilerTest from 'ReactCompilerTest';
6+
7+
function Component() {
8+
return ReactCompilerTest.useHookNotTypedAsHook();
9+
}
10+
11+
```
12+
13+
14+
## Error
15+
16+
```
17+
2 |
18+
3 | function Component() {
19+
> 4 | return ReactCompilerTest.useHookNotTypedAsHook();
20+
| ^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4)
21+
5 | }
22+
6 |
23+
```
24+
25+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import ReactCompilerTest from 'ReactCompilerTest';
2+
3+
function Component() {
4+
return ReactCompilerTest.useHookNotTypedAsHook();
5+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-hook-name-not-typed-as-hook.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function Component() {
1717
2 |
1818
3 | function Component() {
1919
> 4 | return useHookNotTypedAsHook();
20-
| ^^^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for `import {useHookNotTypedAsHook} from 'ReactCompilerTest'` to be a hook based on the exported name (4:4)
20+
| ^^^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4)
2121
5 | }
2222
6 |
2323
```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-type-provider-nonhook-name-typed-as-hook.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function Component() {
1717
2 |
1818
3 | function Component() {
1919
> 4 | return <div>{notAhookTypedAsHook()}</div>;
20-
| ^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for `import {notAhookTypedAsHook} from 'ReactCompilerTest'` not to be a hook based on the exported name (4:4)
20+
| ^^^^^^^^^^^^^^^^^^^ InvalidConfig: Invalid type configuration for module. Expected type for object property 'useHookNotTypedAsHook' from module 'ReactCompilerTest' to be a hook based on the property name (4:4)
2121
5 | }
2222
6 |
2323
```

0 commit comments

Comments
 (0)