Skip to content

Commit 1efb4a8

Browse files
committed
Fix tests: Add dfsEffectsRefactor flag
Some of the tests that gated on the effects refactor used the `new` flag. In order to bisect, we'll need to decompose the new fork changes into multiple steps. So I added a hardcoded test flag called `dfsEffectsRefactor` and set it to false. Will turn back on when we switch back to traversing the finished tree using DFS and `subtreeTag`.
1 parent eb2758b commit 1efb4a8

File tree

6 files changed

+31
-19
lines changed

6 files changed

+31
-19
lines changed

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ describe('ReactDOMServerPartialHydration', () => {
367367
// This is a new node.
368368
expect(span).not.toBe(span2);
369369

370-
if (gate(flags => flags.new)) {
370+
if (gate(flags => flags.dfsEffectsRefactor)) {
371371
// The effects list refactor causes this to be null because the Suspense Offscreen's child
372372
// is null. However, since we can't hydrate Suspense in legacy this change in behavior is ok
373373
expect(ref.current).toBe(null);

packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ beforeEach(() => {
1616
});
1717

1818
// Don't feel too guilty if you have to delete this test.
19+
// @gate dfsEffectsRefactor
1920
// @gate new
2021
// @gate __DEV__
2122
test('warns in DEV if return pointer is inconsistent', async () => {

packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js renamed to packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
'use strict';
1111

1212
let React;
13-
let ReactFeatureFlags;
1413
let ReactTestRenderer;
1514
let Scheduler;
1615
let act;
@@ -19,13 +18,20 @@ describe('ReactDoubleInvokeEvents', () => {
1918
beforeEach(() => {
2019
jest.resetModules();
2120
React = require('react');
22-
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2321
ReactTestRenderer = require('react-test-renderer');
2422
Scheduler = require('scheduler');
25-
ReactFeatureFlags.enableDoubleInvokingEffects = __VARIANT__;
2623
act = ReactTestRenderer.unstable_concurrentAct;
2724
});
2825

26+
function supportsDoubleInvokeEffects() {
27+
return gate(
28+
flags =>
29+
flags.build === 'development' &&
30+
flags.enableDoubleInvokingEffects &&
31+
flags.dfsEffectsRefactor,
32+
);
33+
}
34+
2935
it('should not double invoke effects in legacy mode', () => {
3036
function App({text}) {
3137
React.useEffect(() => {
@@ -73,7 +79,7 @@ describe('ReactDoubleInvokeEvents', () => {
7379
});
7480
});
7581

76-
if (__DEV__ && __VARIANT__) {
82+
if (supportsDoubleInvokeEffects()) {
7783
expect(Scheduler).toHaveYielded([
7884
'useLayoutEffect mount',
7985
'useEffect mount',
@@ -132,7 +138,7 @@ describe('ReactDoubleInvokeEvents', () => {
132138
});
133139
});
134140

135-
if (__DEV__ && __VARIANT__) {
141+
if (supportsDoubleInvokeEffects()) {
136142
expect(Scheduler).toHaveYielded([
137143
'useEffect One mount',
138144
'useEffect Two mount',
@@ -193,7 +199,7 @@ describe('ReactDoubleInvokeEvents', () => {
193199
});
194200
});
195201

196-
if (__DEV__ && __VARIANT__) {
202+
if (supportsDoubleInvokeEffects()) {
197203
expect(Scheduler).toHaveYielded([
198204
'useLayoutEffect One mount',
199205
'useLayoutEffect Two mount',
@@ -250,7 +256,7 @@ describe('ReactDoubleInvokeEvents', () => {
250256
});
251257
});
252258

253-
if (__DEV__ && __VARIANT__) {
259+
if (supportsDoubleInvokeEffects()) {
254260
expect(Scheduler).toHaveYielded([
255261
'useLayoutEffect mount',
256262
'useEffect mount',
@@ -308,7 +314,7 @@ describe('ReactDoubleInvokeEvents', () => {
308314
ReactTestRenderer.create(<App />, {unstable_isConcurrent: true});
309315
});
310316

311-
if (__DEV__ && __VARIANT__) {
317+
if (supportsDoubleInvokeEffects()) {
312318
expect(Scheduler).toHaveYielded([
313319
'componentDidMount',
314320
'componentWillUnmount',
@@ -345,7 +351,7 @@ describe('ReactDoubleInvokeEvents', () => {
345351
});
346352
});
347353

348-
if (__DEV__ && __VARIANT__) {
354+
if (supportsDoubleInvokeEffects()) {
349355
expect(Scheduler).toHaveYielded([
350356
'componentDidMount',
351357
'componentWillUnmount',
@@ -420,7 +426,7 @@ describe('ReactDoubleInvokeEvents', () => {
420426
});
421427
});
422428

423-
if (__DEV__ && __VARIANT__) {
429+
if (supportsDoubleInvokeEffects()) {
424430
expect(Scheduler).toHaveYielded([
425431
'mount',
426432
'useLayoutEffect mount',
@@ -485,7 +491,7 @@ describe('ReactDoubleInvokeEvents', () => {
485491
ReactTestRenderer.create(<App />, {unstable_isConcurrent: true});
486492
});
487493

488-
if (__DEV__ && __VARIANT__) {
494+
if (supportsDoubleInvokeEffects()) {
489495
expect(Scheduler).toHaveYielded([
490496
'App useLayoutEffect mount',
491497
'App useEffect mount',
@@ -505,7 +511,7 @@ describe('ReactDoubleInvokeEvents', () => {
505511
_setShowChild(true);
506512
});
507513

508-
if (__DEV__ && __VARIANT__) {
514+
if (supportsDoubleInvokeEffects()) {
509515
expect(Scheduler).toHaveYielded([
510516
'App useLayoutEffect unmount',
511517
'Child useLayoutEffect mount',
@@ -573,7 +579,7 @@ describe('ReactDoubleInvokeEvents', () => {
573579
});
574580
});
575581

576-
if (__DEV__ && __VARIANT__) {
582+
if (supportsDoubleInvokeEffects()) {
577583
expect(Scheduler).toHaveYielded([
578584
'componentDidMount',
579585
'useLayoutEffect mount',

packages/react/src/__tests__/ReactDOMTracing-test.internal.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ describe('ReactDOMTracing', () => {
152152
onInteractionScheduledWorkCompleted,
153153
).toHaveBeenLastNotifiedOfInteraction(interaction);
154154

155-
if (gate(flags => flags.new)) {
155+
if (gate(flags => flags.dfsEffectsRefactor)) {
156156
expect(onRender).toHaveBeenCalledTimes(3);
157157
} else {
158158
// TODO: This is 4 instead of 3 because this update was scheduled at
@@ -310,7 +310,7 @@ describe('ReactDOMTracing', () => {
310310
expect(
311311
onInteractionScheduledWorkCompleted,
312312
).toHaveBeenLastNotifiedOfInteraction(interaction);
313-
if (gate(flags => flags.new)) {
313+
if (gate(flags => flags.dfsEffectsRefactor)) {
314314
expect(onRender).toHaveBeenCalledTimes(3);
315315
} else {
316316
// TODO: This is 4 instead of 3 because this update was scheduled at

packages/react/src/__tests__/ReactProfiler-test.internal.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ describe('Profiler', () => {
368368

369369
renderer.update(<App />);
370370

371-
if (gate(flags => flags.new)) {
371+
if (gate(flags => flags.dfsEffectsRefactor)) {
372372
// None of the Profiler's subtree was rendered because App bailed out before the Profiler.
373373
// So we expect onRender not to be called.
374374
expect(callback).not.toHaveBeenCalled();
@@ -4292,7 +4292,7 @@ describe('Profiler', () => {
42924292
// because the resolved suspended subtree doesn't contain any passive effects.
42934293
// If <AsyncComponentWithCascadingWork> or its decendents had a passive effect,
42944294
// onPostCommit would be called again.
4295-
if (gate(flags => flags.new)) {
4295+
if (gate(flags => flags.dfsEffectsRefactor)) {
42964296
expect(Scheduler).toFlushAndYield([]);
42974297
} else {
42984298
expect(Scheduler).toFlushAndYield(['onPostCommit']);
@@ -4783,7 +4783,8 @@ describe('Profiler', () => {
47834783
});
47844784

47854785
if (__DEV__) {
4786-
// @gate new
4786+
// @gate dfsEffectsRefactor
4787+
// @gate enableDoubleInvokingEffects
47874788
it('double invoking does not disconnect wrapped async work', () => {
47884789
ReactFeatureFlags.enableDoubleInvokingEffects = true;
47894790

scripts/jest/TestFlags.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ const environmentFlags = {
4444

4545
// Use this for tests that are known to be broken.
4646
FIXME: false,
47+
48+
// Turn this flag back on (or delete) once the effect list is removed in favor
49+
// of a depth-first traversal using `subtreeTags`.
50+
dfsEffectsRefactor: false,
4751
};
4852

4953
function getTestFlags() {

0 commit comments

Comments
 (0)