Skip to content

Commit

Permalink
compiler: only resolve globals and react imports
Browse files Browse the repository at this point in the history
Updates Environment#getGlobalDeclaration() to only resolve "globals" if they are a true global or an import from react/react-dom. We still keep the logic to resolve hook-like names as custom hooks. Notably, this means that a local `Array` reference won't get confused with our Array global declaration, a local `useState` (or import from something other than React) won't get confused as `React.useState()`, etc.

Still working on more tests.

ghstack-source-id: a54ae2a7171fb42a5aad5b6537d6016e66778e4a
Pull Request resolved: #29190
  • Loading branch information
josephsavona committed May 21, 2024
1 parent 912f820 commit 3f36fcd
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { fromZodError } from "zod-validation-error";
import { CompilerError } from "../CompilerError";
import { Logger } from "../Entrypoint";
import { Err, Ok, Result } from "../Utils/Result";
import { log } from "../Utils/logger";
import {
DEFAULT_GLOBALS,
DEFAULT_SHAPES,
Expand Down Expand Up @@ -320,6 +319,8 @@ const EnvironmentConfigSchema = z.object({
*/
throwUnknownException__testonly: z.boolean().default(false),

enableSharedRuntime__testonly: z.boolean().default(false),

/**
* Enables deps of a function epxression to be treated as conditional. This
* makes sure we don't load a dep when it's a property (to check if it has
Expand Down Expand Up @@ -514,7 +515,6 @@ export class Environment {

getGlobalDeclaration(binding: NonLocalBinding): Global | null {
const name = binding.name;
let resolvedName = name;

if (this.config.hookPattern != null) {
const match = new RegExp(this.config.hookPattern).exec(name);
Expand All @@ -523,21 +523,45 @@ export class Environment {
typeof match[1] === "string" &&
isHookName(match[1])
) {
resolvedName = match[1];
const resolvedName = match[1];
return this.#globals.get(resolvedName) ?? this.#getCustomHookType();
}
}

let resolvedGlobal: Global | null = this.#globals.get(resolvedName) ?? null;
if (resolvedGlobal === null) {
// Hack, since we don't track module level declarations and imports
if (isHookName(resolvedName)) {
return this.#getCustomHookType();
} else {
log(() => `Undefined global \`${name}\``);
let global: Global | null = null;
switch (binding.kind) {
case "ModuleLocal": {
// don't resolve module locals
break;
}
case "Global": {
global = this.#globals.get(name) ?? null;
break;
}
case "ImportDefault":
case "ImportNamespace":
case "ImportSpecifier": {
if (
binding.module.toLowerCase() === "react" ||
binding.module.toLowerCase() === "react-dom" ||
(this.config.enableSharedRuntime__testonly &&
binding.module === "shared-runtime")
) {
// only resolve imports to modules we know about
global = this.#globals.get(name) ?? null;
}
break;
}
}

return resolvedGlobal;
if (global === null && isHookName(name)) {
/**
* Type inference relies on all hooks being resolved as such, so if we don't have
* a global declaration and its a hook name, return the default custom hook type.
*/
return this.#getCustomHookType();
}
return global;
}

getPropertyType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,38 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
break;
}
case "LoadGlobal": {
value = `LoadGlobal ${instrValue.binding.name}`;
switch (instrValue.binding.kind) {
case "Global": {
value = `LoadGlobal(global) ${instrValue.binding.name}`;
break;
}
case "ModuleLocal": {
value = `LoadGlobal(module) ${instrValue.binding.name}`;
break;
}
case "ImportDefault": {
value = `LoadGlobal import ${instrValue.binding.name} from '${instrValue.binding.module}'`;
break;
}
case "ImportNamespace": {
value = `LoadGlobal import * as ${instrValue.binding.name} from '${instrValue.binding.module}'`;
break;
}
case "ImportSpecifier": {
if (instrValue.binding.imported !== instrValue.binding.name) {
value = `LoadGlobal import { ${instrValue.binding.imported} as ${instrValue.binding.name} } from '${instrValue.binding.module}'`;
} else {
value = `LoadGlobal import { ${instrValue.binding.name} } from '${instrValue.binding.module}'`;
}
break;
}
default: {
assertExhaustive(
instrValue.binding,
`Unexpected binding kind \`${(instrValue.binding as any).kind}\``
);
}
}
break;
}
case "StoreGlobal": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,71 +56,78 @@ function useFragment(_arg1, _arg2) {
}

function Component(props) {
const $ = _c(14);
const post = useFragment(graphql`...`, props.post);
const $ = _c(15);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = graphql`...`;
$[0] = t0;
} else {
t0 = $[0];
}
const post = useFragment(t0, props.post);
let media;
let allUrls;
let onClick;
if ($[0] !== post) {
if ($[1] !== post) {
allUrls = [];

const { media: t0, comments: t1, urls: t2 } = post;
media = t0 === undefined ? null : t0;
let t3;
if ($[4] !== t1) {
t3 = t1 === undefined ? [] : t1;
$[4] = t1;
$[5] = t3;
} else {
t3 = $[5];
}
const comments = t3;
const { media: t1, comments: t2, urls: t3 } = post;
media = t1 === undefined ? null : t1;
let t4;
if ($[6] !== t2) {
if ($[5] !== t2) {
t4 = t2 === undefined ? [] : t2;
$[6] = t2;
$[7] = t4;
$[5] = t2;
$[6] = t4;
} else {
t4 = $[7];
t4 = $[6];
}
const urls = t4;
const comments = t4;
let t5;
if ($[8] !== comments.length) {
t5 = (e) => {
if ($[7] !== t3) {
t5 = t3 === undefined ? [] : t3;
$[7] = t3;
$[8] = t5;
} else {
t5 = $[8];
}
const urls = t5;
let t6;
if ($[9] !== comments.length) {
t6 = (e) => {
if (!comments.length) {
return;
}

console.log(comments.length);
};
$[8] = comments.length;
$[9] = t5;
$[9] = comments.length;
$[10] = t6;
} else {
t5 = $[9];
t6 = $[10];
}
onClick = t5;
onClick = t6;

allUrls.push(...urls);
$[0] = post;
$[1] = media;
$[2] = allUrls;
$[3] = onClick;
$[1] = post;
$[2] = media;
$[3] = allUrls;
$[4] = onClick;
} else {
media = $[1];
allUrls = $[2];
onClick = $[3];
media = $[2];
allUrls = $[3];
onClick = $[4];
}
let t0;
if ($[10] !== media || $[11] !== allUrls || $[12] !== onClick) {
t0 = <Stringify media={media} allUrls={allUrls} onClick={onClick} />;
$[10] = media;
$[11] = allUrls;
$[12] = onClick;
$[13] = t0;
let t1;
if ($[11] !== media || $[12] !== allUrls || $[13] !== onClick) {
t1 = <Stringify media={media} allUrls={allUrls} onClick={onClick} />;
$[11] = media;
$[12] = allUrls;
$[13] = onClick;
$[14] = t1;
} else {
t0 = $[13];
t1 = $[14];
}
return t0;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@

## Input

```javascript
import { useState as _useState, useCallback, useEffect } from "react";
import { ValidateMemoization } from "shared-runtime";

function useState(value) {
const [state, setState] = _useState(value);
return [state, setState];
}

function Component() {
const [state, setState] = useState("hello");

return <div onClick={() => setState("goodbye")}>{state}</div>;
}

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

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { useState as _useState, useCallback, useEffect } from "react";
import { ValidateMemoization } from "shared-runtime";

function useState(value) {
const $ = _c(5);
let t0;
if ($[0] !== value) {
t0 = _useState(value);
$[0] = value;
$[1] = t0;
} else {
t0 = $[1];
}
const [state, setState] = t0;
let t1;
if ($[2] !== state || $[3] !== setState) {
t1 = [state, setState];
$[2] = state;
$[3] = setState;
$[4] = t1;
} else {
t1 = $[4];
}
return t1;
}

function Component() {
const $ = _c(5);
const [state, setState] = useState("hello");
let t0;
if ($[0] !== setState) {
t0 = () => setState("goodbye");
$[0] = setState;
$[1] = t0;
} else {
t0 = $[1];
}
let t1;
if ($[2] !== t0 || $[3] !== state) {
t1 = <div onClick={t0}>{state}</div>;
$[2] = t0;
$[3] = state;
$[4] = t1;
} else {
t1 = $[4];
}
return t1;
}

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

```
### Eval output
(kind: ok) <div>hello</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { useState as _useState, useCallback, useEffect } from "react";
import { ValidateMemoization } from "shared-runtime";

function useState(value) {
const [state, setState] = _useState(value);
return [state, setState];
}

function Component() {
const [state, setState] = useState("hello");

return <div onClick={() => setState("goodbye")}>{state}</div>;
}

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

0 comments on commit 3f36fcd

Please sign in to comment.