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
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ function runWithEnvironment(
}

if (env.config.validateNoSetStateInEffects) {
env.logErrors(validateNoSetStateInEffects(hir));
env.logErrors(validateNoSetStateInEffects(hir, env));
}

if (env.config.validateNoJSXInTryStatements) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,13 @@ export const EnvironmentConfigSchema = z.object({
* while its parent function remains uncompiled.
*/
validateNoDynamicallyCreatedComponentsOrHooks: z.boolean().default(false),

/**
* When enabled, allows setState calls in effects when the value being set is
* derived from a ref. This is useful for patterns where initial layout measurements
* from refs need to be stored in state during mount.
*/
enableAllowSetStateFromRefsInEffects: z.boolean().default(true),
});

export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -639,12 +639,55 @@ function validateNoRefAccessInRenderImpl(
case 'StartMemoize':
case 'FinishMemoize':
break;
case 'LoadGlobal': {
if (instr.value.binding.name === 'undefined') {
env.set(instr.lvalue.identifier.id, {kind: 'Nullable'});
}
break;
}
case 'Primitive': {
if (instr.value.value == null) {
env.set(instr.lvalue.identifier.id, {kind: 'Nullable'});
}
break;
}
case 'UnaryExpression': {
if (instr.value.operator === '!') {
const value = env.get(instr.value.value.identifier.id);
const refId =
value?.kind === 'RefValue' && value.refId != null
? value.refId
: null;
if (refId !== null) {
/*
* Record an error suggesting the `if (ref.current == null)` pattern,
* but also record the lvalue as a guard so that we don't emit a second
* error for the write to the ref
*/
env.set(instr.lvalue.identifier.id, {kind: 'Guard', refId});
errors.pushDiagnostic(
CompilerDiagnostic.create({
category: ErrorCategory.Refs,
reason: 'Cannot access refs during render',
description: ERROR_DESCRIPTION,
})
.withDetails({
kind: 'error',
loc: instr.value.value.loc,
message: `Cannot access ref value during render`,
})
.withDetails({
kind: 'hint',
message:
'To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`',
}),
);
break;
}
}
validateNoRefValueAccess(errors, env, instr.value.value);
break;
}
case 'BinaryExpression': {
const left = env.get(instr.value.left.identifier.id);
const right = env.get(instr.value.right.identifier.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,23 @@ import {
ErrorCategory,
} from '../CompilerError';
import {
Environment,
HIRFunction,
IdentifierId,
isSetStateType,
isUseEffectHookType,
isUseInsertionEffectHookType,
isUseLayoutEffectHookType,
isUseRefType,
isRefValueType,
Place,
} from '../HIR';
import {eachInstructionValueOperand} from '../HIR/visitors';
import {
eachInstructionLValue,
eachInstructionValueOperand,
} from '../HIR/visitors';
import {Result} from '../Utils/Result';
import {Iterable_some} from '../Utils/utils';

/**
* Validates against calling setState in the body of an effect (useEffect and friends),
Expand All @@ -32,6 +39,7 @@ import {Result} from '../Utils/Result';
*/
export function validateNoSetStateInEffects(
fn: HIRFunction,
env: Environment,
): Result<void, CompilerError> {
const setStateFunctions: Map<IdentifierId, Place> = new Map();
const errors = new CompilerError();
Expand Down Expand Up @@ -72,6 +80,7 @@ export function validateNoSetStateInEffects(
const callee = getSetStateCall(
instr.value.loweredFunc.func,
setStateFunctions,
env,
);
if (callee !== null) {
setStateFunctions.set(instr.lvalue.identifier.id, callee);
Expand Down Expand Up @@ -129,9 +138,42 @@ export function validateNoSetStateInEffects(
function getSetStateCall(
fn: HIRFunction,
setStateFunctions: Map<IdentifierId, Place>,
env: Environment,
): Place | null {
const refDerivedValues: Set<IdentifierId> = new Set();

const isDerivedFromRef = (place: Place): boolean => {
return (
refDerivedValues.has(place.identifier.id) ||
isUseRefType(place.identifier) ||
isRefValueType(place.identifier)
);
};

for (const [, block] of fn.body.blocks) {
for (const instr of block.instructions) {
if (env.config.enableAllowSetStateFromRefsInEffects) {
const hasRefOperand = Iterable_some(
eachInstructionValueOperand(instr.value),
isDerivedFromRef,
);

if (hasRefOperand) {
for (const lvalue of eachInstructionLValue(instr)) {
refDerivedValues.add(lvalue.identifier.id);
}
}

if (
instr.value.kind === 'PropertyLoad' &&
instr.value.property === 'current' &&
(isUseRefType(instr.value.object.identifier) ||
isRefValueType(instr.value.object.identifier))
) {
refDerivedValues.add(instr.lvalue.identifier.id);
}
}

switch (instr.value.kind) {
case 'LoadLocal': {
if (setStateFunctions.has(instr.value.place.identifier.id)) {
Expand Down Expand Up @@ -161,6 +203,21 @@ function getSetStateCall(
isSetStateType(callee.identifier) ||
setStateFunctions.has(callee.identifier.id)
) {
if (env.config.enableAllowSetStateFromRefsInEffects) {
const arg = instr.value.args.at(0);
if (
arg !== undefined &&
arg.kind === 'Identifier' &&
refDerivedValues.has(arg.identifier.id)
) {
/**
* The one special case where we allow setStates in effects is in the very specific
* scenario where the value being set is derived from a ref. For example this may
* be needed when initial layout measurements from refs need to be stored in state.
*/
return null;
}
}
/*
* TODO: once we support multiple locations per error, we should link to the
* original Place in the case that setStateFunction.has(callee)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

## Input

```javascript
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
if (r.current == undefined) {
r.current = 1;
}
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};

```

## Code

```javascript
import { useRef } from "react";

function C() {
const r = useRef(null);
if (r.current == undefined) {
r.current = 1;
}
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};

```

### Eval output
(kind: ok)
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
if (r.current == undefined) {
r.current = 1;
}
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@

## Input

```javascript
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
const current = !r.current;
return <div>{current}</div>;
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};

```


## Error

```
Found 4 errors:

Error: Cannot access refs during render

React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).

4 | component C() {
5 | const r = useRef(null);
> 6 | const current = !r.current;
| ^^^^^^^^^ Cannot access ref value during render
7 | return <div>{current}</div>;
8 | }
9 |

To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`

Error: Cannot access refs during render

React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).

4 | component C() {
5 | const r = useRef(null);
> 6 | const current = !r.current;
| ^^^^^^^^^^ Cannot access ref value during render
7 | return <div>{current}</div>;
8 | }
9 |

Error: Cannot access refs during render

React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).

5 | const r = useRef(null);
6 | const current = !r.current;
> 7 | return <div>{current}</div>;
| ^^^^^^^ Cannot access ref value during render
8 | }
9 |
10 | export const FIXTURE_ENTRYPOINT = {

Error: Cannot access refs during render

React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).

5 | const r = useRef(null);
6 | const current = !r.current;
> 7 | return <div>{current}</div>;
| ^^^^^^^ Cannot access ref value during render
8 | }
9 |
10 | export const FIXTURE_ENTRYPOINT = {
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
const current = !r.current;
return <div>{current}</div>;
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

## Input

```javascript
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
if (!r.current) {
r.current = 1;
}
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};

```


## Error

```
Found 1 error:

Error: Cannot access refs during render

React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).

4 | component C() {
5 | const r = useRef(null);
> 6 | if (!r.current) {
| ^^^^^^^^^ Cannot access ref value during render
7 | r.current = 1;
8 | }
9 | }

To initialize a ref only once, check that the ref is null with the pattern `if (ref.current == null) { ref.current = ... }`
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@flow
import {useRef} from 'react';

component C() {
const r = useRef(null);
if (!r.current) {
r.current = 1;
}
}

export const FIXTURE_ENTRYPOINT = {
fn: C,
params: [{}],
};
Loading
Loading