Skip to content

Commit 57550bb

Browse files
committed
[compiler] Expect component props annotations to be potential objects
Summary: We now expect that candidate components that have Flow or TS type annotations on their first parameters have annotations that are potentially objects--this lets us reject compiling functions that explicitly take e.g. `number` as a parameter. ghstack-source-id: e2c2334 Pull Request resolved: #29866
1 parent 3b9f33a commit 57550bb

File tree

3 files changed

+112
-10
lines changed

3 files changed

+112
-10
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -651,24 +651,75 @@ function isMemoCallback(path: NodePath<t.Expression>): boolean {
651651
);
652652
}
653653

654+
function isValidPropsAnnotation(
655+
annot: t.TypeAnnotation | t.TSTypeAnnotation | t.Noop | null | undefined
656+
): boolean {
657+
if (annot == null) {
658+
return true;
659+
} else if (annot.type === "TSTypeAnnotation") {
660+
switch (annot.typeAnnotation.type) {
661+
case "TSArrayType":
662+
case "TSBigIntKeyword":
663+
case "TSBooleanKeyword":
664+
case "TSConstructorType":
665+
case "TSFunctionType":
666+
case "TSLiteralType":
667+
case "TSNeverKeyword":
668+
case "TSNumberKeyword":
669+
case "TSStringKeyword":
670+
case "TSSymbolKeyword":
671+
case "TSTupleType":
672+
return false;
673+
}
674+
return true;
675+
} else if (annot.type === "TypeAnnotation") {
676+
switch (annot.typeAnnotation.type) {
677+
case "ArrayTypeAnnotation":
678+
case "BooleanLiteralTypeAnnotation":
679+
case "BooleanTypeAnnotation":
680+
case "EmptyTypeAnnotation":
681+
case "FunctionTypeAnnotation":
682+
case "NumberLiteralTypeAnnotation":
683+
case "NumberTypeAnnotation":
684+
case "StringLiteralTypeAnnotation":
685+
case "StringTypeAnnotation":
686+
case "SymbolTypeAnnotation":
687+
case "ThisTypeAnnotation":
688+
case "TupleTypeAnnotation":
689+
return false;
690+
}
691+
return true;
692+
} else if (annot.type === "Noop") {
693+
return true;
694+
} else {
695+
assertExhaustive(annot, `Unexpected annotation node \`${annot}\``);
696+
}
697+
}
698+
654699
function isValidComponentParams(
655700
params: Array<NodePath<t.Identifier | t.Pattern | t.RestElement>>
656701
): boolean {
657702
if (params.length === 0) {
658703
return true;
659-
} else if (params.length === 1) {
660-
return !params[0].isRestElement();
661-
} else if (params.length === 2) {
662-
// check if second param might be a ref
663-
if (params[1].isIdentifier()) {
704+
} else if (params.length > 0 && params.length <= 2) {
705+
if (!isValidPropsAnnotation(params[0].node.typeAnnotation)) {
706+
return false;
707+
}
708+
709+
if (params.length === 1) {
710+
return !params[0].isRestElement();
711+
} else if (params[1].isIdentifier()) {
712+
// check if second param might be a ref
664713
const { name } = params[1].node;
665714
return name.includes("ref") || name.includes("Ref");
715+
} else {
716+
/**
717+
* Otherwise, avoid helper functions that take more than one argument.
718+
* Helpers are _usually_ named with lowercase, but some code may
719+
* violate this rule
720+
*/
721+
return false;
666722
}
667-
/**
668-
* Otherwise, avoid helper functions that take more than one argument.
669-
* Helpers are _usually_ named with lowercase, but some code may
670-
* violate this rule
671-
*/
672723
}
673724
return false;
674725
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @compilationMode(infer)
6+
import { useIdentity, identity } from "shared-runtime";
7+
8+
function Component(fakeProps: number) {
9+
const x = useIdentity(fakeProps);
10+
return identity(x);
11+
}
12+
13+
export const FIXTURE_ENTRYPOINT = {
14+
fn: Component,
15+
params: [42],
16+
};
17+
18+
```
19+
20+
## Code
21+
22+
```javascript
23+
// @compilationMode(infer)
24+
import { useIdentity, identity } from "shared-runtime";
25+
26+
function Component(fakeProps: number) {
27+
const x = useIdentity(fakeProps);
28+
return identity(x);
29+
}
30+
31+
export const FIXTURE_ENTRYPOINT = {
32+
fn: Component,
33+
params: [42],
34+
};
35+
36+
```
37+
38+
### Eval output
39+
(kind: ok) 42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// @compilationMode(infer)
2+
import { useIdentity, identity } from "shared-runtime";
3+
4+
function Component(fakeProps: number) {
5+
const x = useIdentity(fakeProps);
6+
return identity(x);
7+
}
8+
9+
export const FIXTURE_ENTRYPOINT = {
10+
fn: Component,
11+
params: [42],
12+
};

0 commit comments

Comments
 (0)