Skip to content

Commit 1a63add

Browse files
committed
Failing test: Legacy mode sync passive effects
In concurrent roots, if a render is synchronous, we flush its passive effects synchronously. In legacy roots, we don't do this because all updates are synchronous — so we need to flush at the beginning of the next event. This is how `discreteUpdates` worked.
1 parent ed0f38c commit 1a63add

File tree

1 file changed

+35
-1
lines changed

1 file changed

+35
-1
lines changed

packages/react-reconciler/src/__tests__/ReactFlushSync-test.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ describe('ReactFlushSync', () => {
128128
});
129129
});
130130

131-
test('do not flush passive effects synchronously in legacy mode', async () => {
131+
test('do not flush passive effects synchronously after render in legacy mode', async () => {
132132
function App() {
133133
useEffect(() => {
134134
Scheduler.unstable_yieldValue('Effect');
@@ -152,6 +152,40 @@ describe('ReactFlushSync', () => {
152152
expect(Scheduler).toHaveYielded(['Effect']);
153153
});
154154

155+
test('flush pending passive effects before scope is called in legacy mode', async () => {
156+
let currentStep = 0;
157+
158+
function App({step}) {
159+
useEffect(() => {
160+
currentStep = step;
161+
Scheduler.unstable_yieldValue('Effect: ' + step);
162+
}, [step]);
163+
return <Text text={step} />;
164+
}
165+
166+
const root = ReactNoop.createLegacyRoot();
167+
await act(async () => {
168+
ReactNoop.flushSync(() => {
169+
root.render(<App step={1} />);
170+
});
171+
expect(Scheduler).toHaveYielded([
172+
1,
173+
// Because we're in legacy mode, we shouldn't have flushed the passive
174+
// effects yet.
175+
]);
176+
expect(root).toMatchRenderedOutput('1');
177+
178+
ReactNoop.flushSync(() => {
179+
// This should render step 2 because the passive effect has already
180+
// fired, before the scope function is called.
181+
root.render(<App step={currentStep + 1} />);
182+
});
183+
expect(Scheduler).toHaveYielded(['Effect: 1', 2]);
184+
expect(root).toMatchRenderedOutput('2');
185+
});
186+
expect(Scheduler).toHaveYielded(['Effect: 2']);
187+
});
188+
155189
test("do not flush passive effects synchronously when they aren't the result of a sync render", async () => {
156190
function App() {
157191
useEffect(() => {

0 commit comments

Comments
 (0)