mirror of
https://github.com/zebrajr/react.git
synced 2026-01-15 12:15:22 +00:00
[compiler] Don't validate when effect cleanup function depends on effect localized setState state derived values (#35020)
Summary: If we are using a clean up function in an effect and that clean up function depends on a value that is used to set the state we are validating for we shouldn't throw an error since it is a valid use case for an effect. Test Plan: added test --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35020). * #35044 * __->__ #35020
This commit is contained in:
@@ -568,6 +568,26 @@ function renderTree(
|
||||
return result;
|
||||
}
|
||||
|
||||
function getFnLocalDeps(
|
||||
fn: FunctionExpression | undefined,
|
||||
): Set<IdentifierId> | undefined {
|
||||
if (!fn) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const deps: Set<IdentifierId> = new Set();
|
||||
|
||||
for (const [, block] of fn.loweredFunc.func.body.blocks) {
|
||||
for (const instr of block.instructions) {
|
||||
if (instr.value.kind === 'LoadLocal') {
|
||||
deps.add(instr.value.place.identifier.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return deps;
|
||||
}
|
||||
|
||||
function validateEffect(
|
||||
effectFunction: HIRFunction,
|
||||
context: ValidationContext,
|
||||
@@ -586,8 +606,23 @@ function validateEffect(
|
||||
Set<SourceLocation>
|
||||
> = new Map();
|
||||
|
||||
let cleanUpFunctionDeps: Set<IdentifierId> | undefined;
|
||||
|
||||
const globals: Set<IdentifierId> = new Set();
|
||||
for (const block of effectFunction.body.blocks.values()) {
|
||||
/*
|
||||
* if the block is in an effect and is of type return then its an effect's cleanup function
|
||||
* if the cleanup function depends on a value from which effect-set state is derived then
|
||||
* we can't validate
|
||||
*/
|
||||
if (
|
||||
block.terminal.kind === 'return' &&
|
||||
block.terminal.returnVariant === 'Explicit'
|
||||
) {
|
||||
cleanUpFunctionDeps = getFnLocalDeps(
|
||||
context.functions.get(block.terminal.value.identifier.id),
|
||||
);
|
||||
}
|
||||
for (const pred of block.preds) {
|
||||
if (!seenBlocks.has(pred)) {
|
||||
// skip if block has a back edge
|
||||
@@ -698,6 +733,12 @@ function validateEffect(
|
||||
),
|
||||
);
|
||||
|
||||
for (const dep of derivedSetStateCall.sourceIds) {
|
||||
if (cleanUpFunctionDeps !== undefined && cleanUpFunctionDeps.has(dep)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const propsArr = Array.from(propsSet);
|
||||
const stateArr = Array.from(stateSet);
|
||||
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
import {useEffect, useState} from 'react';
|
||||
|
||||
function Component(file: File) {
|
||||
const [imageUrl, setImageUrl] = useState(null);
|
||||
|
||||
/*
|
||||
* Cleaning up the variable or a source of the variable used to setState
|
||||
* inside the effect communicates that we always need to clean up something
|
||||
* which is a valid use case for useEffect. In which case we want to
|
||||
* avoid an throwing
|
||||
*/
|
||||
useEffect(() => {
|
||||
const imageUrlPrepared = URL.createObjectURL(file);
|
||||
setImageUrl(imageUrlPrepared);
|
||||
return () => URL.revokeObjectURL(imageUrlPrepared);
|
||||
}, [file]);
|
||||
|
||||
return <Image src={imageUrl} xstyle={styles.imageSizeLimits} />;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
|
||||
function Component(file) {
|
||||
const $ = _c(5);
|
||||
const [imageUrl, setImageUrl] = useState(null);
|
||||
let t0;
|
||||
let t1;
|
||||
if ($[0] !== file) {
|
||||
t0 = () => {
|
||||
const imageUrlPrepared = URL.createObjectURL(file);
|
||||
setImageUrl(imageUrlPrepared);
|
||||
return () => URL.revokeObjectURL(imageUrlPrepared);
|
||||
};
|
||||
t1 = [file];
|
||||
$[0] = file;
|
||||
$[1] = t0;
|
||||
$[2] = t1;
|
||||
} else {
|
||||
t0 = $[1];
|
||||
t1 = $[2];
|
||||
}
|
||||
useEffect(t0, t1);
|
||||
let t2;
|
||||
if ($[3] !== imageUrl) {
|
||||
t2 = <Image src={imageUrl} xstyle={styles.imageSizeLimits} />;
|
||||
$[3] = imageUrl;
|
||||
$[4] = t2;
|
||||
} else {
|
||||
t2 = $[4];
|
||||
}
|
||||
return t2;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":108},"end":{"line":21,"column":1,"index":700},"filename":"effect-with-cleanup-function-depending-on-derived-computation-value.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
@@ -0,0 +1,21 @@
|
||||
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
import {useEffect, useState} from 'react';
|
||||
|
||||
function Component(file: File) {
|
||||
const [imageUrl, setImageUrl] = useState(null);
|
||||
|
||||
/*
|
||||
* Cleaning up the variable or a source of the variable used to setState
|
||||
* inside the effect communicates that we always need to clean up something
|
||||
* which is a valid use case for useEffect. In which case we want to
|
||||
* avoid an throwing
|
||||
*/
|
||||
useEffect(() => {
|
||||
const imageUrlPrepared = URL.createObjectURL(file);
|
||||
setImageUrl(imageUrlPrepared);
|
||||
return () => URL.revokeObjectURL(imageUrlPrepared);
|
||||
}, [file]);
|
||||
|
||||
return <Image src={imageUrl} xstyle={styles.imageSizeLimits} />;
|
||||
}
|
||||
Reference in New Issue
Block a user