Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/compiler_discord_notify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/runtime_discord_notify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/shared_check_maintainer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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)) {
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/shared_label_core_team_prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -257,6 +258,10 @@ function runWithEnvironment(
validateNoJSXInTryStatement(hir);
}

if (env.config.validateNoImpureFunctionsInRender) {
validateNoImpureFunctionsInRender(hir);
}

inferReactivePlaces(hir);
log({kind: 'hir', name: 'InferReactivePlaces', value: hir});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', [
Expand Down Expand Up @@ -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'}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ export type FunctionSignature = {
* - Else uses the effects specified by this signature.
*/
mutableOnlyIfOperandsAreMutable?: boolean;

impure?: boolean;

canonicalName?: string;
};

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<FunctionTypeConfig> = z.object({
kind: z.literal('function'),
Expand All @@ -50,6 +52,8 @@ export const FunctionTypeSchema: z.ZodType<FunctionTypeConfig> = 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 = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@

## Input

```javascript
// @validateNoImpureFunctionsInRender

function Component() {
const date = Date.now();
const now = performance.now();
const rand = Math.random();
return <Foo date={date} now={now} rand={rand} />;
}

```


## 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 <Foo date={date} now={now} rand={rand} />;
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @validateNoImpureFunctionsInRender

function Component() {
const date = Date.now();
const now = performance.now();
const rand = Math.random();
return <Foo date={date} now={now} rand={rand} />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading