Skip to content

Commit 3b89f99

Browse files
committed
Re-enable skipped test
If synchronous updates are scheduled by a passive effect, that work should be flushed synchronously, even if flushPassiveEffects is called inside batchedUpdates.
1 parent 3bbbb49 commit 3b89f99

File tree

1 file changed

+29
-11
lines changed

1 file changed

+29
-11
lines changed

packages/react-dom/src/__tests__/ReactDOMHooks-test.js renamed to packages/react-dom/src/__tests__/ReactDOMHooks-test.internal.js

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
'use strict';
1111

12+
let ReactFeatureFlags;
13+
let enableNewScheduler;
1214
let React;
1315
let ReactDOM;
1416
let Scheduler;
@@ -19,6 +21,8 @@ describe('ReactDOMHooks', () => {
1921
beforeEach(() => {
2022
jest.resetModules();
2123

24+
ReactFeatureFlags = require('shared/ReactFeatureFlags');
25+
enableNewScheduler = ReactFeatureFlags.enableNewScheduler;
2226
React = require('react');
2327
ReactDOM = require('react-dom');
2428
Scheduler = require('scheduler');
@@ -72,8 +76,7 @@ describe('ReactDOMHooks', () => {
7276
expect(container3.textContent).toBe('6');
7377
});
7478

75-
// TODO: This behavior is wrong. Fix this in the old implementation.
76-
it.skip('can batch synchronous work inside effects with other work', () => {
79+
it('can batch synchronous work inside effects with other work', () => {
7780
let otherContainer = document.createElement('div');
7881

7982
let calledA = false;
@@ -98,15 +101,30 @@ describe('ReactDOMHooks', () => {
98101
}
99102

100103
ReactDOM.render(<Foo />, container);
101-
ReactDOM.unstable_batchedUpdates(() => {
102-
_set(0); // Forces the effect to be flushed
103-
expect(otherContainer.textContent).toBe('');
104-
ReactDOM.render(<B />, otherContainer);
105-
expect(otherContainer.textContent).toBe('');
106-
});
107-
expect(otherContainer.textContent).toBe('B');
108-
expect(calledA).toBe(false); // It was in a batch
109-
expect(calledB).toBe(true);
104+
105+
if (enableNewScheduler) {
106+
// The old behavior was accidental; in the new scheduler, flushing passive
107+
// effects also flushes synchronous work, even inside batchedUpdates.
108+
ReactDOM.unstable_batchedUpdates(() => {
109+
_set(0); // Forces the effect to be flushed
110+
expect(otherContainer.textContent).toBe('A');
111+
ReactDOM.render(<B />, otherContainer);
112+
expect(otherContainer.textContent).toBe('A');
113+
});
114+
expect(otherContainer.textContent).toBe('B');
115+
expect(calledA).toBe(true);
116+
expect(calledB).toBe(true);
117+
} else {
118+
ReactDOM.unstable_batchedUpdates(() => {
119+
_set(0); // Forces the effect to be flushed
120+
expect(otherContainer.textContent).toBe('');
121+
ReactDOM.render(<B />, otherContainer);
122+
expect(otherContainer.textContent).toBe('');
123+
});
124+
expect(otherContainer.textContent).toBe('B');
125+
expect(calledA).toBe(false); // It was in a batch
126+
expect(calledB).toBe(true);
127+
}
110128
});
111129

112130
it('should not bail out when an update is scheduled from within an event handler', () => {

0 commit comments

Comments
 (0)