diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts index 66cde72c4f..be1fef7d59 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts @@ -72,6 +72,7 @@ import { validateContextVariableLValues, validateHooksUsage, validateMemoizedEffectDependencies, + validateNoCapitalizedCalls, validateNoRefAccessInRender, validateNoSetStateInRender, validatePreservedManualMemoization, @@ -154,6 +155,10 @@ function* runWithEnvironment( validateHooksUsage(hir); } + if (env.config.validateNoCapitalizedCalls) { + validateNoCapitalizedCalls(hir); + } + analyseFunctions(hir); yield log({ kind: "hir", name: "AnalyseFunctions", value: hir }); diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts index 0eef92fef7..76f23f315a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts @@ -188,6 +188,18 @@ const EnvironmentConfigSchema = z.object({ */ validateMemoizedEffectDependencies: z.boolean().default(false), + /** + * Validates that there are no capitalized calls other than those allowed by the allowlist. + * Calls to capitalized functions are often functions that used to be components and may + * have lingering hook calls, which makes those calls risky to memoize. + * + * You can specify a list of capitalized calls to allowlist using this option. React Compiler + * always includes its known global functions, including common functions like Boolean and String, + * in this allowlist. You can enable this validation with no additional allowlisted calls by setting + * this option to the empty array. + */ + validateNoCapitalizedCalls: z.nullable(z.array(z.string())).default(null), + /* * When enabled, the compiler assumes that hooks follow the Rules of React: * - Hooks may memoize computation based on any of their parameters, thus @@ -358,6 +370,12 @@ export function parseConfigPragma(pragma: string): EnvironmentConfig { } const keyVal = token.slice(1); let [key, val]: any = keyVal.split(":"); + + if (key === "validateNoCapitalizedCalls") { + maybeConfig[key] = []; + continue; + } + if (typeof defaultConfig[key as keyof EnvironmentConfig] !== "boolean") { // skip parsing non-boolean properties continue; diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoCapitalizedCalls.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoCapitalizedCalls.ts new file mode 100644 index 0000000000..13d49c5be5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoCapitalizedCalls.ts @@ -0,0 +1,61 @@ +/* + * 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, EnvironmentConfig } from ".."; +import { HIRFunction, IdentifierId } from "../HIR"; +import { DEFAULT_GLOBALS } from "../HIR/Globals"; + +export function validateNoCapitalizedCalls(fn: HIRFunction): void { + const envConfig: EnvironmentConfig = fn.env.config; + const ALLOW_LIST = new Set([ + ...DEFAULT_GLOBALS.keys(), + ...(envConfig.validateNoCapitalizedCalls ?? []), + ]); + /* + * The hook pattern may allow uppercase names, like React$useState, so we need to be sure that we + * do not error in those cases + */ + const hookPattern = + envConfig.hookPattern != null ? new RegExp(envConfig.hookPattern) : null; + const isAllowed = (name: string): boolean => { + return ( + ALLOW_LIST.has(name) || (hookPattern != null && hookPattern.test(name)) + ); + }; + + const capitalLoadGlobals = new Map(); + for (const [, block] of fn.body.blocks) { + for (const { lvalue, value } of block.instructions) { + switch (value.kind) { + case "LoadGlobal": { + if ( + value.name != "" && + /^[A-Z]/.test(value.name) && + // We don't want to flag CONSTANTS() + !(value.name.toUpperCase() === value.name) && + !isAllowed(value.name) + ) { + capitalLoadGlobals.set(lvalue.identifier.id, value.name); + } + + break; + } + case "CallExpression": { + const calleeIdentifier = value.callee.identifier.id; + const calleeName = capitalLoadGlobals.get(calleeIdentifier); + if (calleeName != null) { + CompilerError.throwInvalidReact({ + reason: `Capitalized function calls may be calling components that use hooks, which make them dangerous to memoize. Ensure there are no hook calls in the function and rename it to begin with a lowercase letter to fix this error`, + description: `${calleeName} may be a component.`, + loc: value.loc, + suggestions: null, + }); + } + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/index.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/index.ts index a6e30b4812..f745b7f626 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/index.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/index.ts @@ -8,6 +8,7 @@ export { validateContextVariableLValues } from "./ValidateContextVariableLValues"; export { validateHooksUsage } from "./ValidateHooksUsage"; export { validateMemoizedEffectDependencies } from "./ValidateMemoizedEffectDependencies"; +export { validateNoCapitalizedCalls } from "./ValidateNoCapitalizedCalls"; export { validateNoRefAccessInRender } from "./ValidateNoRefAccesInRender"; export { validateNoSetStateInRender } from "./ValidateNoSetStateInRender"; export { validatePreservedManualMemoization } from "./ValidatePreservedManualMemoization"; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.expect.md new file mode 100644 index 0000000000..9b73080c76 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.expect.md @@ -0,0 +1,53 @@ + +## Input + +```javascript +// @validateNoCapitalizedCalls @hookPattern:".*\b(use[^$]+)$" +import * as React from "react"; +const React$useState = React.useState; +const THIS_IS_A_CONSTANT = () => {}; +function Component() { + const b = Boolean(true); // OK + const n = Number(3); // OK + const s = String("foo"); // OK + const [state, setState] = React$useState(0); // OK + const [state2, setState2] = React.useState(1); // OK + const constant = THIS_IS_A_CONSTANT(); // OK + return 3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; + +``` + +## Code + +```javascript +// @validateNoCapitalizedCalls @hookPattern:".*\b(use[^$]+)$" +import * as React from "react"; +const React$useState = React.useState; +const THIS_IS_A_CONSTANT = () => {}; +function Component() { + Boolean(true); + Number(3); + String("foo"); + React$useState(0); + React.useState(1); + THIS_IS_A_CONSTANT(); + return 3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok) 3 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.js new file mode 100644 index 0000000000..34c44913a7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capitalized-function-allowlist.js @@ -0,0 +1,19 @@ +// @validateNoCapitalizedCalls @hookPattern:".*\b(use[^$]+)$" +import * as React from "react"; +const React$useState = React.useState; +const THIS_IS_A_CONSTANT = () => {}; +function Component() { + const b = Boolean(true); // OK + const n = Number(3); // OK + const s = String("foo"); // OK + const [state, setState] = React$useState(0); // OK + const [state2, setState2] = React.useState(1); // OK + const constant = THIS_IS_A_CONSTANT(); // OK + return 3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + isComponent: true, +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.expect.md new file mode 100644 index 0000000000..a7e6e81e75 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.expect.md @@ -0,0 +1,20 @@ + +## Input + +```javascript +// @validateNoCapitalizedCalls +function Foo() { + let x = Bar; + x(); // ERROR +} + +``` + + +## Error + +``` +[ReactForget] InvalidReact: Capitalized function calls may be calling components that use hooks, which make them dangerous to memoize. Ensure there are no hook calls in the function and rename it to begin with a lowercase letter to fix this error. Bar may be a component. (4:4) +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.js new file mode 100644 index 0000000000..17659c5c32 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call-aliased.js @@ -0,0 +1,5 @@ +// @validateNoCapitalizedCalls +function Foo() { + let x = Bar; + x(); // ERROR +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.expect.md new file mode 100644 index 0000000000..53d30f422d --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.expect.md @@ -0,0 +1,21 @@ + +## Input + +```javascript +// @validateNoCapitalizedCalls +function Component() { + const x = SomeFunc(); + + return x; +} + +``` + + +## Error + +``` +[ReactForget] InvalidReact: Capitalized function calls may be calling components that use hooks, which make them dangerous to memoize. Ensure there are no hook calls in the function and rename it to begin with a lowercase letter to fix this error. SomeFunc may be a component. (3:3) +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.js new file mode 100644 index 0000000000..bfae64ac9e --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.capitalized-function-call.js @@ -0,0 +1,6 @@ +// @validateNoCapitalizedCalls +function Component() { + const x = SomeFunc(); + + return x; +}