diff --git a/compiler/forget/packages/snap/src/compiler-worker.ts b/compiler/forget/packages/snap/src/compiler-worker.ts index cf35707069..cc33a06507 100644 --- a/compiler/forget/packages/snap/src/compiler-worker.ts +++ b/compiler/forget/packages/snap/src/compiler-worker.ts @@ -113,7 +113,7 @@ export async function compile( }, ], ]), - inlineUseMemo: true, + validateHooksUsage: true, }, logger: null, gating, diff --git a/compiler/forget/src/CompilerPipeline.ts b/compiler/forget/src/CompilerPipeline.ts index fba60ba925..210a320698 100644 --- a/compiler/forget/src/CompilerPipeline.ts +++ b/compiler/forget/src/CompilerPipeline.ts @@ -13,6 +13,7 @@ import { mergeConsecutiveBlocks, ReactiveFunction, validateConsistentIdentifiers, + validateHooksUsage, validateTerminalSuccessors, } from "./HIR"; import { Environment, EnvironmentConfig } from "./HIR/Environment"; @@ -73,17 +74,21 @@ export function* run( enterSSA(hir); yield log({ kind: "hir", name: "SSA", value: hir }); - validateConsistentIdentifiers(hir); - eliminateRedundantPhi(hir); yield log({ kind: "hir", name: "EliminateRedundantPhi", value: hir }); + validateConsistentIdentifiers(hir); + constantPropagation(hir); yield log({ kind: "hir", name: "ConstantPropagation", value: hir }); inferTypes(hir); yield log({ kind: "hir", name: "InferTypes", value: hir }); + if (env.validateHooksUsage) { + validateHooksUsage(hir); + } + dropMemoCalls(hir); yield log({ kind: "hir", name: "DropMemoCalls", value: hir }); diff --git a/compiler/forget/src/HIR/Environment.ts b/compiler/forget/src/HIR/Environment.ts index 4e11375a52..fc8b587b53 100644 --- a/compiler/forget/src/HIR/Environment.ts +++ b/compiler/forget/src/HIR/Environment.ts @@ -19,11 +19,11 @@ import { Effect, FunctionType, IdentifierId, - makeBlockId, - makeIdentifierId, ObjectType, PolyType, ValueKind, + makeBlockId, + makeIdentifierId, } from "./HIR"; import { Hook } from "./Hooks"; import { FunctionSignature, ShapeRegistry } from "./ObjectShape"; @@ -40,6 +40,7 @@ const HOOK_PATTERN = /^_?use/; export type EnvironmentConfig = Partial<{ customHooks: Map; memoizeJsxElements: boolean; + validateHooksUsage: boolean; }>; export class Environment { @@ -47,6 +48,7 @@ export class Environment { #shapes: ShapeRegistry; #nextIdentifer: number = 0; #nextBlock: number = 0; + validateHooksUsage: boolean; constructor(config: EnvironmentConfig | null) { this.#shapes = DEFAULT_SHAPES; @@ -66,6 +68,7 @@ export class Environment { } else { this.#globals = DEFAULT_GLOBALS; } + this.validateHooksUsage = config?.validateHooksUsage ?? false; } get nextIdentifierId(): IdentifierId { diff --git a/compiler/forget/src/HIR/ValidateHooksUsage.ts b/compiler/forget/src/HIR/ValidateHooksUsage.ts new file mode 100644 index 0000000000..f1134e9edc --- /dev/null +++ b/compiler/forget/src/HIR/ValidateHooksUsage.ts @@ -0,0 +1,90 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { + CompilerError, + CompilerErrorDetail, + ErrorSeverity, +} from "../CompilerError"; +import { hasBackEdge } from "../Optimization/DeadCodeElimination"; +import { HIRFunction, IdentifierId, Place, isHookType } from "./HIR"; +import { eachInstructionValueOperand, eachTerminalOperand } from "./visitors"; + +/** + * Validates that the function honors the [Rules of Hooks](https://react.dev/warnings/invalid-hook-call-warning) + * rule that hooks may only be called and not otherwise referenced as first-class values. + */ +export function validateHooksUsage(fn: HIRFunction): void { + const errors = new CompilerError(); + const pushError = (place: Place): void => { + errors.pushErrorDetail( + new CompilerErrorDetail({ + codeframe: null, + description: null, + reason: + "Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning)", + loc: typeof place.loc !== "symbol" ? place.loc : null, + severity: ErrorSeverity.InvalidInput, + }) + ); + }; + + const hooks: Set = new Set(); + const hasLoop = hasBackEdge(fn); + + let size = hooks.size; + do { + size = hooks.size; + for (const [, block] of fn.body.blocks) { + for (const phi of block.phis) { + let possibleHook = false; + for (const [, predecessor] of phi.operands) { + if (hooks.has(predecessor.id)) { + possibleHook = true; + break; + } + } + if (possibleHook) { + hooks.add(phi.id.id); + } + } + + for (const instr of block.instructions) { + if ( + instr.value.kind === "LoadGlobal" && + isHookType(instr.lvalue.identifier) + ) { + hooks.add(instr.lvalue.identifier.id); + } else if (instr.value.kind === "CallExpression") { + for (const operand of eachInstructionValueOperand(instr.value)) { + if (operand === instr.value.callee) { + continue; + } + if (hooks.has(operand.identifier.id)) { + pushError(operand); + } + } + } else { + for (const operand of eachInstructionValueOperand(instr.value)) { + if (hooks.has(operand.identifier.id)) { + pushError(operand); + } + } + } + } + for (const operand of eachTerminalOperand(block.terminal)) { + if (hooks.has(operand.identifier.id)) { + pushError(operand); + } + } + } + } while (hooks.size > size && hasLoop); + + if (errors.hasErrors()) { + throw errors; + } +} diff --git a/compiler/forget/src/HIR/index.ts b/compiler/forget/src/HIR/index.ts index 92d345d71e..82ac925eff 100644 --- a/compiler/forget/src/HIR/index.ts +++ b/compiler/forget/src/HIR/index.ts @@ -19,4 +19,5 @@ export { Hook } from "./Hooks"; export { mergeConsecutiveBlocks } from "./MergeConsecutiveBlocks"; export { printFunction, printHIR } from "./PrintHIR"; export { validateConsistentIdentifiers } from "./ValidateConsistentIdentifiers"; +export { validateHooksUsage } from "./ValidateHooksUsage"; export { validateTerminalSuccessors } from "./ValidateTerminalSuccessors"; diff --git a/compiler/forget/src/Optimization/DeadCodeElimination.ts b/compiler/forget/src/Optimization/DeadCodeElimination.ts index bc6aabafe9..1f747b2815 100644 --- a/compiler/forget/src/Optimization/DeadCodeElimination.ts +++ b/compiler/forget/src/Optimization/DeadCodeElimination.ts @@ -247,7 +247,7 @@ function pruneableValue(value: InstructionValue, state: State): boolean { } } -function hasBackEdge(fn: HIRFunction): boolean { +export function hasBackEdge(fn: HIRFunction): boolean { const visited = new Set(); for (const [blockId, block] of fn.body.blocks) { for (const predId of block.preds) { diff --git a/compiler/forget/src/__tests__/compiler-test.ts b/compiler/forget/src/__tests__/compiler-test.ts index 4e1a48d6fd..ffe40a03ee 100644 --- a/compiler/forget/src/__tests__/compiler-test.ts +++ b/compiler/forget/src/__tests__/compiler-test.ts @@ -55,6 +55,7 @@ describe("React Forget", () => { }, ], ]), + validateHooksUsage: true, }, logger: null, gating: options.gating, diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.expect.md new file mode 100644 index 0000000000..398153b4a5 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.expect.md @@ -0,0 +1,20 @@ + +## Input + +```javascript +function Component(props) { + const x = useRef; + const ref = x(null); + return ref.current; +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/useRef-rename-mutable.js b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.js similarity index 100% rename from compiler/forget/src/__tests__/fixtures/compiler/useRef-rename-mutable.js rename to compiler/forget/src/__tests__/fixtures/compiler/error.invalid-assign-hook-to-local.js diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.expect.md new file mode 100644 index 0000000000..8812ae2822 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.expect.md @@ -0,0 +1,18 @@ + +## Input + +```javascript +function Component(props) { + return foo(useFoo); +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.js b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.js new file mode 100644 index 0000000000..b53bcb8181 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-call-arg.js @@ -0,0 +1,3 @@ +function Component(props) { + return foo(useFoo); +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.expect.md new file mode 100644 index 0000000000..bc0594ffb9 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.expect.md @@ -0,0 +1,18 @@ + +## Input + +```javascript +function Component(props) { + return ; +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.js b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.js new file mode 100644 index 0000000000..4b3ea52d74 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-pass-hook-as-prop.js @@ -0,0 +1,3 @@ +function Component(props) { + return ; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.expect.md new file mode 100644 index 0000000000..d37443b981 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.expect.md @@ -0,0 +1,21 @@ + +## Input + +```javascript +function Component(props) { + const x = props.cond ? useA : useB; + return x(); +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2) + +[ReactForget] InvalidInput: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (2:2) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.js b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.js new file mode 100644 index 0000000000..3a47b73e1c --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.invalid-ternary-with-hook-values.js @@ -0,0 +1,4 @@ +function Component(props) { + const x = props.cond ? useA : useB; + return x(); +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/useRef-rename-mutable.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/useRef-rename-mutable.expect.md deleted file mode 100644 index 3526bdc374..0000000000 --- a/compiler/forget/src/__tests__/fixtures/compiler/useRef-rename-mutable.expect.md +++ /dev/null @@ -1,22 +0,0 @@ - -## Input - -```javascript -function Component(props) { - const x = useRef; - const ref = x(null); - return ref.current; -} - -``` - -## Code - -```javascript -function Component(props) { - const ref = useRef(null); - return ref.current; -} - -``` - \ No newline at end of file