Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
into: signatureArgument(4),
signature: null,
mutatesFunction: false,
loc: GeneratedSource,
},
// captures the result of the callback into the return array
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,12 @@ export function printAliasingEffect(effect: AliasingEffect): string {
case 'MutateGlobal': {
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
}
case 'Impure': {
return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
}
case 'Render': {
return `Render ${printPlaceForAliasEffect(effect.place)}`;
}
default: {
assertExhaustive(effect, `Unexpected kind '${(effect as any).kind}'`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
capturedOrMutated.add(effect.value.identifier.id);
break;
}
case 'Impure':
case 'Render':
case 'MutateFrozen':
case 'MutateGlobal':
case 'CreateFunction':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
CompilerErrorDetailOptions,
Effect,
ErrorSeverity,
SourceLocation,
ValueKind,
} from '..';
import {
Expand Down Expand Up @@ -380,10 +381,7 @@ function applySignature(
context: new Set(),
});
effects.push({
kind:
value.kind === ValueKind.Frozen
? 'MutateFrozen'
: 'MutateGlobal',
kind: 'MutateFrozen',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
Expand Down Expand Up @@ -546,7 +544,10 @@ function applyEffect(
const hasTrackedSideEffects =
effect.function.loweredFunc.func.aliasingEffects?.some(
effect =>
effect.kind === 'MutateFrozen' || effect.kind === 'MutateGlobal',
// TODO; include "render" here?
effect.kind === 'MutateFrozen' ||
effect.kind === 'MutateGlobal' ||
effect.kind === 'Impure',
);
// For legacy compatibility
const capturesRef = effect.function.loweredFunc.func.context.some(
Expand Down Expand Up @@ -721,6 +722,7 @@ function applyEffect(
effect.receiver,
effect.args,
functionValues[0].loweredFunc.func.context,
effect.loc,
);
if (signatureEffects != null) {
if (DEBUG) {
Expand All @@ -747,6 +749,8 @@ function applyEffect(
effect.into,
effect.receiver,
effect.args,
[],
effect.loc,
)
: null;
if (signatureEffects != null) {
Expand All @@ -766,6 +770,7 @@ function applyEffect(
effect.into,
effect.receiver,
effect.args,
effect.loc,
);
for (const legacyEffect of legacyEffects) {
applyEffect(context, state, legacyEffect, aliased, effects);
Expand Down Expand Up @@ -899,6 +904,8 @@ function applyEffect(
}
break;
}
case 'Impure':
case 'Render':
case 'MutateFrozen':
case 'MutateGlobal': {
effects.push(effect);
Expand Down Expand Up @@ -1452,6 +1459,7 @@ function computeSignatureForInstruction(
args: value.args,
into: lvalue,
signature,
loc: value.loc,
});
break;
}
Expand Down Expand Up @@ -1631,6 +1639,24 @@ function computeSignatureForInstruction(
into: lvalue,
});
}
if (value.kind === 'JsxExpression') {
if (value.tag.kind === 'Identifier') {
// Tags are render function, by definition they're called during render
effects.push({
kind: 'Render',
place: value.tag,
});
}
if (value.children != null) {
// Children are typically called during render, not used as an event/effect callback
for (const child of value.children) {
effects.push({
kind: 'Render',
place: child,
});
}
}
}
break;
}
case 'DeclareLocal': {
Expand Down Expand Up @@ -1861,6 +1887,7 @@ function computeEffectsForLegacySignature(
lvalue: Place,
receiver: Place,
args: Array<Place | SpreadPattern | Hole>,
loc: SourceLocation,
): Array<AliasingEffect> {
const returnValueReason = signature.returnValueReason ?? ValueReason.Other;
const effects: Array<AliasingEffect> = [];
Expand All @@ -1870,6 +1897,23 @@ function computeEffectsForLegacySignature(
value: signature.returnValueKind,
reason: returnValueReason,
});
if (signature.impure && state.env.config.validateNoImpureFunctionsInRender) {
effects.push({
kind: 'Impure',
place: receiver,
error: {
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,
suggestions: null,
},
});
}
const stores: Array<Place> = [];
const captures: Array<Place> = [];
function visit(place: Place, effect: Effect): void {
Expand Down Expand Up @@ -2083,6 +2127,7 @@ function computeEffectsForSignature(
args: Array<Place | SpreadPattern | Hole>,
// Used for signatures constructed dynamically which reference context variables
context: Array<Place> = [],
loc: SourceLocation,
): Array<AliasingEffect> | null {
if (
// Not enough args
Expand Down Expand Up @@ -2163,6 +2208,7 @@ function computeEffectsForSignature(
}
break;
}
case 'Impure':
case 'MutateFrozen':
case 'MutateGlobal': {
const values = substitutions.get(effect.place.identifier.id) ?? [];
Expand All @@ -2171,6 +2217,13 @@ function computeEffectsForSignature(
}
break;
}
case 'Render': {
const values = substitutions.get(effect.place.identifier.id) ?? [];
for (const value of values) {
effects.push({kind: effect.kind, place: value});
}
break;
}
case 'Mutate':
case 'MutateTransitive':
case 'MutateTransitiveConditionally':
Expand Down Expand Up @@ -2254,6 +2307,7 @@ function computeEffectsForSignature(
function: applyFunction[0],
into: applyInto[0],
signature: effect.signature,
loc,
});
break;
}
Expand Down Expand Up @@ -2468,6 +2522,7 @@ export type AliasingEffect =
args: Array<Place | SpreadPattern | Hole>;
into: Place;
signature: FunctionSignature | null;
loc: SourceLocation;
}
/**
* Constructs a function value with the given captures. The mutability of the function
Expand All @@ -2490,6 +2545,20 @@ export type AliasingEffect =
kind: 'MutateGlobal';
place: Place;
error: CompilerErrorDetailOptions;
}
/**
* Indicates a side-effect that is not safe during render
*/
| {kind: 'Impure'; place: Place; error: CompilerErrorDetailOptions}
/**
* Indicates that a given place is accessed during render. Used to distingush
* hook arguments that are known to be called immediately vs those used for
* event handlers/effects, and for JSX values known to be called during render
* (tags, children) vs those that may be events/effect (other props).
*/
| {
kind: 'Render';
place: Place;
};

function hashEffect(effect: AliasingEffect): string {
Expand Down Expand Up @@ -2536,6 +2605,8 @@ function hashEffect(effect: AliasingEffect): string {
case 'Freeze': {
return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
}
case 'Impure':
case 'Render':
case 'MutateFrozen':
case 'MutateGlobal': {
return [effect.kind, effect.place.identifier.id].join(':');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ export function inferMutationAliasingFunctionEffects(
]);
} else if (
effect.kind === 'MutateFrozen' ||
effect.kind === 'MutateGlobal'
effect.kind === 'MutateGlobal' ||
effect.kind === 'Impure' ||
effect.kind === 'Render'
) {
effects.push(effect);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export function inferMutationAliasingRanges(
kind: MutationKind;
place: Place;
}> = [];
const renders: Array<{index: number; place: Place}> = [];

let index = 0;

Expand Down Expand Up @@ -160,9 +161,12 @@ export function inferMutationAliasingRanges(
});
} else if (
effect.kind === 'MutateFrozen' ||
effect.kind === 'MutateGlobal'
effect.kind === 'MutateGlobal' ||
effect.kind === 'Impure'
) {
errors.push(effect.error);
} else if (effect.kind === 'Render') {
renders.push({index: index++, place: effect.place});
}
}
}
Expand Down Expand Up @@ -209,6 +213,9 @@ export function inferMutationAliasingRanges(
errors,
);
}
for (const render of renders) {
state.render(render.index, render.place.identifier, errors);
}
if (DEBUG) {
console.log(pretty([...state.nodes.keys()]));
}
Expand Down Expand Up @@ -357,6 +364,8 @@ export function inferMutationAliasingRanges(
// no-op, Read is the default
break;
}
case 'Impure':
case 'Render':
case 'MutateFrozen':
case 'MutateGlobal': {
// no-op
Expand Down Expand Up @@ -440,6 +449,7 @@ export function inferMutationAliasingRanges(
function appendFunctionErrors(errors: CompilerError, fn: HIRFunction): void {
for (const effect of fn.aliasingEffects ?? []) {
switch (effect.kind) {
case 'Impure':
case 'MutateFrozen':
case 'MutateGlobal': {
errors.push(effect.error);
Expand Down Expand Up @@ -530,6 +540,43 @@ class AliasingState {
}
}

render(index: number, start: Identifier, errors: CompilerError): void {
const seen = new Set<Identifier>();
const queue: Array<Identifier> = [start];
while (queue.length !== 0) {
const current = queue.pop()!;
if (seen.has(current)) {
continue;
}
seen.add(current);
const node = this.nodes.get(current);
if (node == null || node.transitive != null || node.local != null) {
continue;
}
if (node.value.kind === 'Function') {
appendFunctionErrors(errors, node.value.function);
}
for (const [alias, when] of node.createdFrom) {
if (when >= index) {
continue;
}
queue.push(alias);
}
for (const [alias, when] of node.aliases) {
if (when >= index) {
continue;
}
queue.push(alias);
}
for (const [capture, when] of node.captures) {
if (when >= index) {
continue;
}
queue.push(capture);
}
}
}

mutate(
index: number,
start: Identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
## Input

```javascript
// @enableNewMutationAliasingModel:false
function Component() {
const foo = () => {
someGlobal = true;
Expand All @@ -15,13 +16,13 @@ function Component() {
## Error

```
1 | function Component() {
2 | const foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
4 | };
5 | return <div {...foo} />;
6 | }
2 | function Component() {
3 | const foo = () => {
> 4 | someGlobal = true;
| ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4)
5 | };
6 | return <div {...foo} />;
7 | }
```


Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @enableNewMutationAliasingModel:false
function Component() {
const foo = () => {
someGlobal = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function Component() {
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). `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)

Expand Down
Loading
Loading