Compare commits

..

2 Commits

Author SHA1 Message Date
Jorge Cabiedes Acosta
c10461285a [compiler] Switch to track setStates by aliasing and id instead of identifier names
Summary:
This makes the setState usage logic much more robust. We no longer rely on identifierName.

Now we track when a setState is loaded into a new promoted identifier variable and track this in a map `setStateLoaded` map.

For other types of instructions we consider the setState to be being used. In this case we record its usage into the `setStateUsages` map.



Test Plan:
We expect no changes in behavior for the current tests
2025-11-10 12:10:22 -08:00
Jorge Cabiedes Acosta
4c9faa3ec3 [compiler] Update ValidateNoDerivedComputationsInEffects_exp to log the error instead of throwing
Summary:
TSIA

Simple change to log errors in Pipeline.ts instead of throwing in the validation

Test Plan:
updated snap tests
2025-11-10 12:10:22 -08:00
6 changed files with 8 additions and 673 deletions

View File

@@ -358,14 +358,8 @@ function recordInstructionDerivations(
context.effects.add(effectFunction.loweredFunc.func);
}
} else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
typeOfValue = 'fromState';
context.derivationCache.addDerivationEntry(
lvalue,
new Set(),
typeOfValue,
true,
);
return;
isSource = true;
typeOfValue = joinValue(typeOfValue, 'fromState');
}
}
@@ -574,26 +568,6 @@ 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,
@@ -612,23 +586,8 @@ 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
@@ -739,12 +698,6 @@ 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);

View File

@@ -1,76 +0,0 @@
## 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

View File

@@ -1,21 +0,0 @@
// @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} />;
}

View File

@@ -1,72 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
function Component({prop}) {
const [s, setS] = useState();
const [second, setSecond] = useState(prop);
/*
* `second` is a source of state. It will inherit the value of `prop` in
* the first render, but after that it will no longer be updated when
* `prop` changes. So we shouldn't consider `second` as being derived from
* `prop`
*/
useEffect(() => {
setS(second);
}, [second]);
return <div>{s}</div>;
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
function Component(t0) {
const $ = _c(5);
const { prop } = t0;
const [s, setS] = useState();
const [second] = useState(prop);
let t1;
let t2;
if ($[0] !== second) {
t1 = () => {
setS(second);
};
t2 = [second];
$[0] = second;
$[1] = t1;
$[2] = t2;
} else {
t1 = $[1];
t2 = $[2];
}
useEffect(t1, t2);
let t3;
if ($[3] !== s) {
t3 = <div>{s}</div>;
$[3] = s;
$[4] = t3;
} else {
t3 = $[4];
}
return t3;
}
```
## Logs
```
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [second]\n\nData Flow Tree:\n└── second (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":14,"column":4,"index":443},"end":{"line":14,"column":8,"index":447},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":18,"column":1,"index":500},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
```
### Eval output
(kind: exception) Fixture not implemented

View File

@@ -1,18 +0,0 @@
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
function Component({prop}) {
const [s, setS] = useState();
const [second, setSecond] = useState(prop);
/*
* `second` is a source of state. It will inherit the value of `prop` in
* the first render, but after that it will no longer be updated when
* `prop` changes. So we shouldn't consider `second` as being derived from
* `prop`
*/
useEffect(() => {
setS(second);
}, [second]);
return <div>{s}</div>;
}

View File

@@ -10,17 +10,11 @@
'use strict';
let React;
let ReactDOM;
let ReactDOMClient;
let Scheduler;
let act;
let Activity;
let useState;
let useLayoutEffect;
let useEffect;
let LegacyHidden;
let assertLog;
let Suspense;
let ReactDOM;
let ReactDOMClient;
let act;
describe('ReactDOMActivity', () => {
let container;
@@ -28,19 +22,11 @@ describe('ReactDOMActivity', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
Scheduler = require('scheduler/unstable_mock');
Activity = React.Activity;
useState = React.useState;
Suspense = React.Suspense;
useState = React.useState;
LegacyHidden = React.unstable_LegacyHidden;
useLayoutEffect = React.useLayoutEffect;
useEffect = React.useEffect;
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
const InternalTestUtils = require('internal-test-utils');
act = InternalTestUtils.act;
assertLog = InternalTestUtils.assertLog;
act = require('internal-test-utils').act;
container = document.createElement('div');
document.body.appendChild(container);
});
@@ -49,11 +35,6 @@ describe('ReactDOMActivity', () => {
document.body.removeChild(container);
});
function Text(props) {
Scheduler.log(props.text);
return <span prop={props.text}>{props.children}</span>;
}
// @gate enableActivity
it(
'hiding an Activity boundary also hides the direct children of any ' +
@@ -72,7 +53,7 @@ describe('ReactDOMActivity', () => {
);
}
function App() {
function App({portalContents}) {
return (
<Accordion>
<div>
@@ -118,7 +99,7 @@ describe('ReactDOMActivity', () => {
);
}
function App() {
function App({portalContents}) {
return (
<Activity mode="hidden">
<div>
@@ -150,416 +131,4 @@ describe('ReactDOMActivity', () => {
);
},
);
// @gate enableActivity
it('hides new portals added to an already hidden tree', async () => {
function Child() {
return <Text text="Child" />;
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Mount hidden tree.
await act(() => {
root.render(
<Activity mode="hidden">
<Text text="Parent" />
</Activity>,
);
});
assertLog(['Parent']);
expect(container.innerHTML).toBe(
'<span prop="Parent" style="display: none;"></span>',
);
expect(portalContainer.innerHTML).toBe('');
// Add a portal inside the hidden tree.
await act(() => {
root.render(
<Activity mode="hidden">
<Text text="Parent" />
<Portal>
<Child />
</Portal>
</Activity>,
);
});
assertLog(['Parent', 'Child']);
expect(container.innerHTML).toBe(
'<span prop="Parent" style="display: none;"></span><div style="display: none;"></div>',
);
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span>',
);
// Now reveal it.
await act(() => {
root.render(
<Activity mode="visible">
<Text text="Parent" />
<Portal>
<Child />
</Portal>
</Activity>,
);
});
assertLog(['Parent', 'Child']);
expect(container.innerHTML).toBe(
'<span prop="Parent" style=""></span><div style=""></div>',
);
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style=""></span>',
);
});
// @gate enableActivity
it('hides new insertions inside an already hidden portal', async () => {
function Child({text}) {
useLayoutEffect(() => {
Scheduler.log(`Mount layout ${text}`);
return () => {
Scheduler.log(`Unmount layout ${text}`);
};
}, [text]);
return <Text text={text} />;
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Mount hidden tree.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Child text="A" />
</Portal>
</Activity>,
);
});
assertLog(['A']);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="A" style="display: none;"></span>',
);
// Add a node inside the hidden portal.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Child text="A" />
<Child text="B" />
</Portal>
</Activity>,
);
});
assertLog(['A', 'B']);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="A" style="display: none;"></span><span prop="B" style="display: none;"></span>',
);
// Now reveal it.
await act(() => {
root.render(
<Activity mode="visible">
<Portal>
<Child text="A" />
<Child text="B" />
</Portal>
</Activity>,
);
});
assertLog(['A', 'B', 'Mount layout A', 'Mount layout B']);
expect(container.innerHTML).toBe('<div style=""></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="A" style=""></span><span prop="B" style=""></span>',
);
});
// @gate enableActivity
it('reveal an inner Suspense boundary without revealing an outer Activity on the same host child', async () => {
const promise = new Promise(() => {});
function Child({showInner}) {
useLayoutEffect(() => {
Scheduler.log('Mount layout');
return () => {
Scheduler.log('Unmount layout');
};
}, []);
return (
<>
{showInner ? null : promise}
<Text text="Child" />
</>
);
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Prerender the whole tree.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Suspense name="Inner" fallback={<span>Loading</span>}>
<Child showInner={true} />
</Suspense>
</Portal>
</Activity>,
);
});
assertLog(['Child']);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span>',
);
// Re-suspend the inner.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Suspense name="Inner" fallback={<span>Loading</span>}>
<Child showInner={false} />
</Suspense>
</Portal>
</Activity>,
);
});
assertLog([]);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span><span style="display: none;">Loading</span>',
);
// Toggle to visible while suspended.
await act(() => {
root.render(
<Activity mode="visible">
<Portal>
<Suspense name="Inner" fallback={<span>Loading</span>}>
<Child showInner={false} />
</Suspense>
</Portal>
</Activity>,
);
});
assertLog([]);
expect(container.innerHTML).toBe('<div style=""></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span><span style="">Loading</span>',
);
// Now reveal.
await act(() => {
root.render(
<Activity mode="visible">
<Portal>
<Suspense name="Inner" fallback={<span>Loading</span>}>
<Child showInner={true} />
</Suspense>
</Portal>
</Activity>,
);
});
assertLog(['Child', 'Mount layout']);
expect(container.innerHTML).toBe('<div style=""></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style=""></span>',
);
});
// @gate enableActivity
it('mounts/unmounts layout effects in portal when visibility changes (starting visible)', async () => {
function Child() {
useLayoutEffect(() => {
Scheduler.log('Mount layout');
return () => {
Scheduler.log('Unmount layout');
};
}, []);
return <Text text="Child" />;
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Mount visible tree.
await act(() => {
root.render(
<Activity mode="visible">
<Portal>
<Child />
</Portal>
</Activity>,
);
});
assertLog(['Child', 'Mount layout']);
expect(container.innerHTML).toBe('<div></div>');
expect(portalContainer.innerHTML).toBe('<span prop="Child"></span>');
// Hide the tree. The layout effect is unmounted.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Child />
</Portal>
</Activity>,
);
});
assertLog(['Unmount layout', 'Child']);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span>',
);
});
// @gate enableActivity
it('mounts/unmounts layout effects in portal when visibility changes (starting hidden)', async () => {
function Child() {
useLayoutEffect(() => {
Scheduler.log('Mount layout');
return () => {
Scheduler.log('Unmount layout');
};
}, []);
return <Text text="Child" />;
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Mount hidden tree.
await act(() => {
root.render(
<Activity mode="hidden">
<Portal>
<Child />
</Portal>
</Activity>,
);
});
// No layout effect.
assertLog(['Child']);
expect(container.innerHTML).toBe('<div style="display: none;"></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style="display: none;"></span>',
);
// Unhide the tree. The layout effect is mounted.
await act(() => {
root.render(
<Activity mode="visible">
<Portal>
<Child />
</Portal>
</Activity>,
);
});
assertLog(['Child', 'Mount layout']);
expect(container.innerHTML).toBe('<div style=""></div>');
expect(portalContainer.innerHTML).toBe(
'<span prop="Child" style=""></span>',
);
});
// @gate enableLegacyHidden
it('does not toggle effects or hide nodes for LegacyHidden component inside portal', async () => {
function Child() {
useLayoutEffect(() => {
Scheduler.log('Mount layout');
return () => {
Scheduler.log('Unmount layout');
};
}, []);
useEffect(() => {
Scheduler.log('Mount passive');
return () => {
Scheduler.log('Unmount passive');
};
}, []);
return <Text text="Child" />;
}
const portalContainer = document.createElement('div');
function Portal({children}) {
return <div>{ReactDOM.createPortal(children, portalContainer)}</div>;
}
const root = ReactDOMClient.createRoot(container);
// Mount visible tree.
await act(() => {
root.render(
<LegacyHidden mode="visible">
<Portal>
<Child />
</Portal>
</LegacyHidden>,
);
});
assertLog(['Child', 'Mount layout', 'Mount passive']);
expect(container.innerHTML).toBe('<div></div>');
expect(portalContainer.innerHTML).toBe('<span prop="Child"></span>');
// Hide the tree.
await act(() => {
root.render(
<LegacyHidden mode="hidden">
<Portal>
<Child />
</Portal>
</LegacyHidden>,
);
});
// Effects not unmounted.
assertLog(['Child']);
expect(container.innerHTML).toBe('<div></div>');
expect(portalContainer.innerHTML).toBe('<span prop="Child"></span>');
// Unhide the tree.
await act(() => {
root.render(
<LegacyHidden mode="visible">
<Portal>
<Child />
</Portal>
</LegacyHidden>,
);
});
// Effects already mounted.
assertLog(['Child']);
expect(container.innerHTML).toBe('<div></div>');
expect(portalContainer.innerHTML).toBe('<span prop="Child"></span>');
});
});