Skip to content

Commit

Permalink
[compiler] validation against calling impure functions (#31960)
Browse files Browse the repository at this point in the history
For now we just reject all calls of impure functions, and the validation
is off by default. Going forward we can make this more precise and only
reject impure functions called during render.

Note that I was intentionally imprecise in the return type of these
functions in order to avoid changing output of existing code. We lie to
the compiler and say that Date.now, performance.now, and Math.random
return unknown mutable objects rather than primitives. Once the
validation is complete and vetted we can switch this to be more precise.
  • Loading branch information
josephsavona authored Jan 17, 2025
1 parent 313c8c5 commit 7f3826e
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 0 deletions.
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} />;
}

0 comments on commit 7f3826e

Please sign in to comment.