diff --git a/.github/workflows/compiler_discord_notify.yml b/.github/workflows/compiler_discord_notify.yml index b9cc3e9b9fa83..b4694e12be94d 100644 --- a/.github/workflows/compiler_discord_notify.yml +++ b/.github/workflows/compiler_discord_notify.yml @@ -10,9 +10,11 @@ on: jobs: check_maintainer: uses: facebook/react/.github/workflows/shared_check_maintainer.yml@main + with: + actor: ${{ github.event.pull_request.user.login }} notify: - if: ${{ needs.check_maintainer.outputs.is_core_team }} + if: ${{ needs.check_maintainer.outputs.is_core_team == 'true' }} needs: check_maintainer runs-on: ubuntu-latest steps: diff --git a/.github/workflows/runtime_discord_notify.yml b/.github/workflows/runtime_discord_notify.yml index abf7970a1e634..a29735ac4ed8e 100644 --- a/.github/workflows/runtime_discord_notify.yml +++ b/.github/workflows/runtime_discord_notify.yml @@ -10,9 +10,11 @@ on: jobs: check_maintainer: uses: facebook/react/.github/workflows/shared_check_maintainer.yml@main + with: + actor: ${{ github.event.pull_request.user.login }} notify: - if: ${{ needs.check_maintainer.outputs.is_core_team }} + if: ${{ needs.check_maintainer.outputs.is_core_team == 'true' }} needs: check_maintainer runs-on: ubuntu-latest steps: diff --git a/.github/workflows/shared_check_maintainer.yml b/.github/workflows/shared_check_maintainer.yml index 53f4c7a8af046..cd43ffe55979a 100644 --- a/.github/workflows/shared_check_maintainer.yml +++ b/.github/workflows/shared_check_maintainer.yml @@ -2,6 +2,10 @@ name: (Shared) Check maintainer on: workflow_call: + inputs: + actor: + required: true + type: string outputs: is_core_team: value: ${{ jobs.check_maintainer.outputs.is_core_team }} @@ -24,7 +28,7 @@ jobs: with: script: | const fs = require('fs'); - const actor = '${{ github.actor }}'; + const actor = '${{ inputs.actor }}'; const data = await fs.readFileSync('./MAINTAINERS', { encoding: 'utf8' }); const maintainers = new Set(data.split('\n')); if (maintainers.has(actor)) { diff --git a/.github/workflows/shared_label_core_team_prs.yml b/.github/workflows/shared_label_core_team_prs.yml index b96aea88054fa..dc432b54f72a6 100644 --- a/.github/workflows/shared_label_core_team_prs.yml +++ b/.github/workflows/shared_label_core_team_prs.yml @@ -11,9 +11,11 @@ env: jobs: check_maintainer: uses: facebook/react/.github/workflows/shared_check_maintainer.yml@main + with: + actor: ${{ github.event.pull_request.user.login }} label: - if: ${{ needs.check_maintainer.outputs.is_core_team }} + if: ${{ needs.check_maintainer.outputs.is_core_team == 'true' }} runs-on: ubuntu-latest needs: check_maintainer steps: diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index ca6abc0748eb9..a354acf69274e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -99,6 +99,7 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI import {outlineJSX} from '../Optimization/OutlineJsx'; import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; import {transformFire} from '../Transform'; +import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -257,6 +258,10 @@ function runWithEnvironment( validateNoJSXInTryStatement(hir); } + if (env.config.validateNoImpureFunctionsInRender) { + validateNoImpureFunctionsInRender(hir); + } + inferReactivePlaces(hir); log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index f1db567660ec9..9df34242ec089 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -353,6 +353,11 @@ const EnvironmentConfigSchema = z.object({ validateNoCapitalizedCalls: z.nullable(z.array(z.string())).default(null), validateBlocklistedImports: z.nullable(z.array(z.string())).default(null), + /** + * Validate against impure functions called during render + */ + validateNoImpureFunctionsInRender: z.boolean().default(false), + /* * When enabled, the compiler assumes that hooks follow the Rules of React: * - Hooks may memoize computation based on any of their parameters, thus diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index ac5d44bab0875..3fe2e938ce57b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -141,6 +141,44 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ ], ]), ], + [ + 'performance', + addObject(DEFAULT_SHAPES, 'performance', [ + // Static methods (TODO) + [ + 'now', + // Date.now() + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'performance.now', + }), + ], + ]), + ], + [ + 'Date', + addObject(DEFAULT_SHAPES, 'Date', [ + // Static methods (TODO) + [ + 'now', + // Date.now() + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'Date.now', + }), + ], + ]), + ], [ 'Math', addObject(DEFAULT_SHAPES, 'Math', [ @@ -209,6 +247,18 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ returnValueKind: ValueKind.Primitive, }), ], + [ + 'random', + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Poly'}, // TODO: could be Primitive, but that would change existing compilation + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, // same here + impure: true, + canonicalName: 'Math.random', + }), + ], ]), ], ['Infinity', {kind: 'Primitive'}], diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 4482d17890196..7c05c05531ae9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -172,6 +172,10 @@ export type FunctionSignature = { * - Else uses the effects specified by this signature. */ mutableOnlyIfOperandsAreMutable?: boolean; + + impure?: boolean; + + canonicalName?: string; }; /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts index 362328db72155..9aac2a264f60a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/TypeSchema.ts @@ -40,6 +40,8 @@ export type FunctionTypeConfig = { returnValueKind: ValueKind; noAlias?: boolean | null | undefined; mutableOnlyIfOperandsAreMutable?: boolean | null | undefined; + impure?: boolean | null | undefined; + canonicalName?: string | null | undefined; }; export const FunctionTypeSchema: z.ZodType = z.object({ kind: z.literal('function'), @@ -50,6 +52,8 @@ export const FunctionTypeSchema: z.ZodType = z.object({ returnValueKind: ValueKindSchema, noAlias: z.boolean().nullable().optional(), mutableOnlyIfOperandsAreMutable: z.boolean().nullable().optional(), + impure: z.boolean().nullable().optional(), + canonicalName: z.string().nullable().optional(), }); export type HookTypeConfig = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts new file mode 100644 index 0000000000000..287a0aa978b6f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValiateNoImpureFunctionsInRender.ts @@ -0,0 +1,52 @@ +/** + * 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, ErrorSeverity} from '..'; +import {HIRFunction} from '../HIR'; +import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; + +/** + * Checks that known-impure functions are not called during render. Examples of invalid functions to + * call during render are `Math.random()` and `Date.now()`. Users may extend this set of + * impure functions via a module type provider and specifying functions with `impure: true`. + * + * TODO: add best-effort analysis of functions which are called during render. We have variations of + * this in several of our validation passes and should unify those analyses into a reusable helper + * and use it here. + */ +export function validateNoImpureFunctionsInRender(fn: HIRFunction): void { + const errors = new CompilerError(); + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const value = instr.value; + if (value.kind === 'MethodCall' || value.kind == 'CallExpression') { + const callee = + value.kind === 'MethodCall' ? value.property : value.callee; + const signature = getFunctionCallSignature( + fn.env, + callee.identifier.type, + ); + if (signature != null && signature.impure === true) { + errors.push({ + reason: + 'Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)', + description: + signature.canonicalName != null + ? `\`${signature.canonicalName}\` is an impure function whose results may change on every call` + : null, + severity: ErrorSeverity.InvalidReact, + loc: callee.loc, + suggestions: null, + }); + } + } + } + } + if (errors.hasErrors()) { + throw errors; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md new file mode 100644 index 0000000000000..a67d467df8cfd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +// @validateNoImpureFunctionsInRender + +function Component() { + const date = Date.now(); + const now = performance.now(); + const rand = Math.random(); + return ; +} + +``` + + +## Error + +``` + 2 | + 3 | function Component() { +> 4 | const date = Date.now(); + | ^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4) + +InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `performance.now` is an impure function whose results may change on every call (5:5) + +InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Math.random` is an impure function whose results may change on every call (6:6) + 5 | const now = performance.now(); + 6 | const rand = Math.random(); + 7 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js new file mode 100644 index 0000000000000..6faf98caff702 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.js @@ -0,0 +1,8 @@ +// @validateNoImpureFunctionsInRender + +function Component() { + const date = Date.now(); + const now = performance.now(); + const rand = Math.random(); + return ; +} diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index af93760ccfcc9..cea03b73b59d5 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -780,7 +780,7 @@ const supportsMoveBefore = // $FlowFixMe[prop-missing]: We're doing the feature detection here. enableMoveBefore && typeof window !== 'undefined' && - typeof window.Node.prototype.moveBefore === 'function'; + typeof window.Element.prototype.moveBefore === 'function'; export function appendChild( parentInstance: Instance,