Skip to content

Commit 0f5741c

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 0f5741c

File tree

6 files changed

+31
-18
lines changed

6 files changed

+31
-18
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

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,20 @@ describe('ReactDoubleInvokeEvents', () => {
1919
beforeEach(() => {
2020
jest.resetModules();
2121
React = require('react');
22-
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2322
ReactTestRenderer = require('react-test-renderer');
2423
Scheduler = require('scheduler');
25-
ReactFeatureFlags.enableDoubleInvokingEffects = __VARIANT__;
2624
act = ReactTestRenderer.unstable_concurrentAct;
2725
});
2826

27+
function supportsDoubleInvokeEffects() {
28+
return gate(
29+
flags =>
30+
flags.build === 'development' &&
31+
flags.enableDoubleInvokingEffects &&
32+
flags.dfsEffectsRefactor,
33+
);
34+
}
35+
2936
it('should not double invoke effects in legacy mode', () => {
3037
function App({text}) {
3138
React.useEffect(() => {
@@ -73,7 +80,7 @@ describe('ReactDoubleInvokeEvents', () => {
7380
});
7481
});
7582

76-
if (__DEV__ && __VARIANT__) {
83+
if (supportsDoubleInvokeEffects()) {
7784
expect(Scheduler).toHaveYielded([
7885
'useLayoutEffect mount',
7986
'useEffect mount',
@@ -132,7 +139,7 @@ describe('ReactDoubleInvokeEvents', () => {
132139
});
133140
});
134141

135-
if (__DEV__ && __VARIANT__) {
142+
if (supportsDoubleInvokeEffects()) {
136143
expect(Scheduler).toHaveYielded([
137144
'useEffect One mount',
138145
'useEffect Two mount',
@@ -193,7 +200,7 @@ describe('ReactDoubleInvokeEvents', () => {
193200
});
194201
});
195202

196-
if (__DEV__ && __VARIANT__) {
203+
if (supportsDoubleInvokeEffects()) {
197204
expect(Scheduler).toHaveYielded([
198205
'useLayoutEffect One mount',
199206
'useLayoutEffect Two mount',
@@ -250,7 +257,7 @@ describe('ReactDoubleInvokeEvents', () => {
250257
});
251258
});
252259

253-
if (__DEV__ && __VARIANT__) {
260+
if (supportsDoubleInvokeEffects()) {
254261
expect(Scheduler).toHaveYielded([
255262
'useLayoutEffect mount',
256263
'useEffect mount',
@@ -308,7 +315,7 @@ describe('ReactDoubleInvokeEvents', () => {
308315
ReactTestRenderer.create(<App />, {unstable_isConcurrent: true});
309316
});
310317

311-
if (__DEV__ && __VARIANT__) {
318+
if (supportsDoubleInvokeEffects()) {
312319
expect(Scheduler).toHaveYielded([
313320
'componentDidMount',
314321
'componentWillUnmount',
@@ -345,7 +352,7 @@ describe('ReactDoubleInvokeEvents', () => {
345352
});
346353
});
347354

348-
if (__DEV__ && __VARIANT__) {
355+
if (supportsDoubleInvokeEffects()) {
349356
expect(Scheduler).toHaveYielded([
350357
'componentDidMount',
351358
'componentWillUnmount',
@@ -420,7 +427,7 @@ describe('ReactDoubleInvokeEvents', () => {
420427
});
421428
});
422429

423-
if (__DEV__ && __VARIANT__) {
430+
if (supportsDoubleInvokeEffects()) {
424431
expect(Scheduler).toHaveYielded([
425432
'mount',
426433
'useLayoutEffect mount',
@@ -485,7 +492,7 @@ describe('ReactDoubleInvokeEvents', () => {
485492
ReactTestRenderer.create(<App />, {unstable_isConcurrent: true});
486493
});
487494

488-
if (__DEV__ && __VARIANT__) {
495+
if (supportsDoubleInvokeEffects()) {
489496
expect(Scheduler).toHaveYielded([
490497
'App useLayoutEffect mount',
491498
'App useEffect mount',
@@ -505,7 +512,7 @@ describe('ReactDoubleInvokeEvents', () => {
505512
_setShowChild(true);
506513
});
507514

508-
if (__DEV__ && __VARIANT__) {
515+
if (supportsDoubleInvokeEffects()) {
509516
expect(Scheduler).toHaveYielded([
510517
'App useLayoutEffect unmount',
511518
'Child useLayoutEffect mount',
@@ -573,7 +580,7 @@ describe('ReactDoubleInvokeEvents', () => {
573580
});
574581
});
575582

576-
if (__DEV__ && __VARIANT__) {
583+
if (supportsDoubleInvokeEffects()) {
577584
expect(Scheduler).toHaveYielded([
578585
'componentDidMount',
579586
'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)