Skip to content

Commit

Permalink
[compiler] Expect component props annotations to be potential objects
Browse files Browse the repository at this point in the history
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: 03622a68d62bb7a4029dd8cf228e62b290b6dd7b
Pull Request resolved: #29866
  • Loading branch information
mvitousek committed Jun 11, 2024
1 parent 4666faf commit ab87e8e
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -651,18 +651,69 @@ function isMemoCallback(path: NodePath<t.Expression>): boolean {
);
}

function isValidPropsAnnotation(
annot: t.TypeAnnotation | t.TSTypeAnnotation | t.Noop | null | undefined
): boolean {
if (annot == null) {
return true;
} else if (annot.type === "TSTypeAnnotation") {
switch (annot.typeAnnotation.type) {
case "TSArrayType":
case "TSBigIntKeyword":
case "TSBooleanKeyword":
case "TSConstructorType":
case "TSFunctionType":
case "TSLiteralType":
case "TSNeverKeyword":
case "TSNumberKeyword":
case "TSStringKeyword":
case "TSSymbolKeyword":
case "TSTupleType":
return false;
}
return true;
} else if (annot.type === "TypeAnnotation") {
switch (annot.typeAnnotation.type) {
case "ArrayTypeAnnotation":
case "BooleanLiteralTypeAnnotation":
case "BooleanTypeAnnotation":
case "EmptyTypeAnnotation":
case "FunctionTypeAnnotation":
case "NumberLiteralTypeAnnotation":
case "NumberTypeAnnotation":
case "StringLiteralTypeAnnotation":
case "StringTypeAnnotation":
case "SymbolTypeAnnotation":
case "ThisTypeAnnotation":
case "TupleTypeAnnotation":
return false;
}
return true;
} else if (annot.type === "Noop") {
return true;
} else {
assertExhaustive(annot, `Unexpected annotation node \`${annot}\``);
}
}

function isValidComponentParams(
params: Array<NodePath<t.Identifier | t.Pattern | t.RestElement>>
): boolean {
if (params.length === 0) {
return true;
} else if (params.length === 1) {
return !params[0].isRestElement();
return (
isValidPropsAnnotation(params[0].node.typeAnnotation) &&
!params[0].isRestElement()
);
} else if (params.length === 2) {
// check if second param might be a ref
if (params[1].isIdentifier()) {
const { name } = params[1].node;
return name.includes("ref") || name.includes("Ref");
return (
isValidPropsAnnotation(params[0].node.typeAnnotation) &&
(name.includes("ref") || name.includes("Ref"))
);
}
/**
* Otherwise, avoid helper functions that take more than one argument.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@

## Input

```javascript
// @compilationMode(infer)
import { useIdentity, identity } from "shared-runtime";

function Component(fakeProps: number) {
const x = useIdentity(fakeProps);
return identity(x);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [42],
};

```

## Code

```javascript
// @compilationMode(infer)
import { useIdentity, identity } from "shared-runtime";

function Component(fakeProps: number) {
const x = useIdentity(fakeProps);
return identity(x);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [42],
};

```
### Eval output
(kind: ok) 42
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// @compilationMode(infer)
import { useIdentity, identity } from "shared-runtime";

function Component(fakeProps: number) {
const x = useIdentity(fakeProps);
return identity(x);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [42],
};

0 comments on commit ab87e8e

Please sign in to comment.