Skip to content

Commit

Permalink
Remove forceConcurrentByDefaultForTesting flag (#30436)
Browse files Browse the repository at this point in the history
Concurrent by default has been unshipped! Let's clean it up.

Here we remove `forceConcurrentByDefaultForTesting`, which allows us to
run tests against both concurrent strategies. In the next PR, we'll
remove the actual concurrent by default code path.
  • Loading branch information
jackpope authored Jul 24, 2024
1 parent b943feb commit e902c45
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 365 deletions.
62 changes: 1 addition & 61 deletions packages/react-art/src/__tests__/ReactART-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,15 @@ import Circle from 'react-art/Circle';
import Rectangle from 'react-art/Rectangle';
import Wedge from 'react-art/Wedge';

const {act, waitFor} = require('internal-test-utils');
const {act} = require('internal-test-utils');

// Isolate DOM renderer.
jest.resetModules();
// share isomorphic
jest.mock('scheduler', () => Scheduler);
jest.mock('react', () => React);
const ReactDOM = require('react-dom');
const ReactDOMClient = require('react-dom/client');

// Isolate the noop renderer
jest.resetModules();
// share isomorphic
jest.mock('scheduler', () => Scheduler);
jest.mock('react', () => React);
const ReactNoop = require('react-noop-renderer');

let Group;
let Shape;
let Surface;
Expand Down Expand Up @@ -397,58 +389,6 @@ describe('ReactART', () => {
doClick(instance);
expect(onClick2).toBeCalled();
});

// @gate forceConcurrentByDefaultForTesting
it('can concurrently render with a "primary" renderer while sharing context', async () => {
const CurrentRendererContext = React.createContext(null);

function Yield(props) {
Scheduler.log(props.value);
return null;
}

let ops = [];
function LogCurrentRenderer() {
return (
<CurrentRendererContext.Consumer>
{currentRenderer => {
ops.push(currentRenderer);
return null;
}}
</CurrentRendererContext.Consumer>
);
}

ReactNoop.render(
<CurrentRendererContext.Provider value="Test">
<Yield value="A" />
<Yield value="B" />
<LogCurrentRenderer />
<Yield value="C" />
</CurrentRendererContext.Provider>,
);

await waitFor(['A']);

const root = ReactDOMClient.createRoot(container);
// We use flush sync here because we expect this to render in between
// while the concurrent render is yieldy where as act would flush both.
ReactDOM.flushSync(() => {
root.render(
<Surface>
<LogCurrentRenderer />
<CurrentRendererContext.Provider value="ART">
<LogCurrentRenderer />
</CurrentRendererContext.Provider>
</Surface>,
);
});

ops = [];
await waitFor(['B', 'C']);

expect(ops).toEqual(['Test']);
});
});

describe('ReactARTComponents', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
expect(container.textContent).toEqual('not hovered');

await waitFor(['hovered']);
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
expect(container.textContent).toEqual('not hovered');
} else {
expect(container.textContent).toEqual('hovered');
}
expect(container.textContent).toEqual('hovered');
});
expect(container.textContent).toEqual('hovered');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2551,14 +2551,7 @@ describe('ReactDOMServerPartialHydration', () => {
suspend = true;

await act(async () => {
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
await waitFor(['Before']);
// This took a long time to render.
Scheduler.unstable_advanceTime(1000);
await waitFor(['After']);
} else {
await waitFor(['Before', 'After']);
}
await waitFor(['Before', 'After']);

// This will cause us to skip the second row completely.
});
Expand Down
6 changes: 0 additions & 6 deletions packages/react-reconciler/src/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
enableProfilerTimer,
enableScopeAPI,
enableLegacyHidden,
forceConcurrentByDefaultForTesting,
allowConcurrentByDefault,
enableTransitionTracing,
enableDebugTracing,
Expand Down Expand Up @@ -534,11 +533,6 @@ export function createHostRootFiber(
mode |= StrictLegacyMode | StrictEffectsMode;
}
if (
// We only use this flag for our repo tests to check both behaviors.
forceConcurrentByDefaultForTesting
) {
mode |= ConcurrentUpdatesByDefaultMode;
} else if (
// Only for internal experiments.
allowConcurrentByDefault &&
concurrentUpdatesByDefaultOverride
Expand Down
133 changes: 43 additions & 90 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,54 +115,28 @@ describe('ReactExpiration', () => {
}
}

function flushNextRenderIfExpired() {
// This will start rendering the next level of work. If the work hasn't
// expired yet, React will exit without doing anything. If it has expired,
// it will schedule a sync task.
Scheduler.unstable_flushExpired();
// Flush the sync task.
ReactNoop.flushSync();
}

it('increases priority of updates as time progresses', async () => {
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
ReactNoop.render(<span prop="done" />);
expect(ReactNoop).toMatchRenderedOutput(null);

// Nothing has expired yet because time hasn't advanced.
flushNextRenderIfExpired();
expect(ReactNoop).toMatchRenderedOutput(null);
// Advance time a bit, but not enough to expire the low pri update.
ReactNoop.expire(4500);
flushNextRenderIfExpired();
expect(ReactNoop).toMatchRenderedOutput(null);
// Advance by another second. Now the update should expire and flush.
ReactNoop.expire(500);
flushNextRenderIfExpired();
expect(ReactNoop).toMatchRenderedOutput(<span prop="done" />);
} else {
ReactNoop.render(<Text text="Step 1" />);
React.startTransition(() => {
ReactNoop.render(<Text text="Step 2" />);
});
await waitFor(['Step 1']);
ReactNoop.render(<Text text="Step 1" />);
React.startTransition(() => {
ReactNoop.render(<Text text="Step 2" />);
});
await waitFor(['Step 1']);

expect(ReactNoop).toMatchRenderedOutput('Step 1');
expect(ReactNoop).toMatchRenderedOutput('Step 1');

// Nothing has expired yet because time hasn't advanced.
await unstable_waitForExpired([]);
expect(ReactNoop).toMatchRenderedOutput('Step 1');
// Nothing has expired yet because time hasn't advanced.
await unstable_waitForExpired([]);
expect(ReactNoop).toMatchRenderedOutput('Step 1');

// Advance time a bit, but not enough to expire the low pri update.
ReactNoop.expire(4500);
await unstable_waitForExpired([]);
expect(ReactNoop).toMatchRenderedOutput('Step 1');
// Advance time a bit, but not enough to expire the low pri update.
ReactNoop.expire(4500);
await unstable_waitForExpired([]);
expect(ReactNoop).toMatchRenderedOutput('Step 1');

// Advance by a little bit more. Now the update should expire and flush.
ReactNoop.expire(500);
await unstable_waitForExpired(['Step 2']);
expect(ReactNoop).toMatchRenderedOutput('Step 2');
}
// Advance by a little bit more. Now the update should expire and flush.
ReactNoop.expire(500);
await unstable_waitForExpired(['Step 2']);
expect(ReactNoop).toMatchRenderedOutput('Step 2');
});

it('two updates of like priority in the same event always flush within the same batch', async () => {
Expand Down Expand Up @@ -408,57 +382,36 @@ describe('ReactExpiration', () => {
jest.resetModules();
Scheduler = require('scheduler');

if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
// Before importing the renderer, advance the current time by a number
// larger than the maximum allowed for bitwise operations.
const maxSigned31BitInt = 1073741823;
Scheduler.unstable_advanceTime(maxSigned31BitInt * 100);
// Now import the renderer. On module initialization, it will read the
// current time.
ReactNoop = require('react-noop-renderer');
ReactNoop.render('Hi');
const InternalTestUtils = require('internal-test-utils');
waitFor = InternalTestUtils.waitFor;
assertLog = InternalTestUtils.assertLog;
unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired;

// The update should not have expired yet.
flushNextRenderIfExpired();
await waitFor([]);
expect(ReactNoop).toMatchRenderedOutput(null);
// Advance the time some more to expire the update.
Scheduler.unstable_advanceTime(10000);
flushNextRenderIfExpired();
await waitFor([]);
expect(ReactNoop).toMatchRenderedOutput('Hi');
} else {
const InternalTestUtils = require('internal-test-utils');
waitFor = InternalTestUtils.waitFor;
assertLog = InternalTestUtils.assertLog;
unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired;

// Before importing the renderer, advance the current time by a number
// larger than the maximum allowed for bitwise operations.
const maxSigned31BitInt = 1073741823;
Scheduler.unstable_advanceTime(maxSigned31BitInt * 100);

// Now import the renderer. On module initialization, it will read the
// current time.
ReactNoop = require('react-noop-renderer');
React = require('react');

ReactNoop.render(<Text text="Step 1" />);
React.startTransition(() => {
ReactNoop.render(<Text text="Step 2" />);
});
await waitFor(['Step 1']);
// Before importing the renderer, advance the current time by a number
// larger than the maximum allowed for bitwise operations.
const maxSigned31BitInt = 1073741823;
Scheduler.unstable_advanceTime(maxSigned31BitInt * 100);

// Now import the renderer. On module initialization, it will read the
// current time.
ReactNoop = require('react-noop-renderer');
React = require('react');

ReactNoop.render(<Text text="Step 1" />);
React.startTransition(() => {
ReactNoop.render(<Text text="Step 2" />);
});
await waitFor(['Step 1']);

// The update should not have expired yet.
await unstable_waitForExpired([]);
// The update should not have expired yet.
await unstable_waitForExpired([]);

expect(ReactNoop).toMatchRenderedOutput('Step 1');
expect(ReactNoop).toMatchRenderedOutput('Step 1');

// Advance the time some more to expire the update.
Scheduler.unstable_advanceTime(10000);
await unstable_waitForExpired(['Step 2']);
expect(ReactNoop).toMatchRenderedOutput('Step 2');
}
// Advance the time some more to expire the update.
Scheduler.unstable_advanceTime(10000);
await unstable_waitForExpired(['Step 2']);
expect(ReactNoop).toMatchRenderedOutput('Step 2');
});

it('should measure callback timeout relative to current time, not start-up time', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1540,39 +1540,21 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: (empty)" />);

// Rendering again should flush the previous commit's effects
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
React.startTransition(() => {
ReactNoop.render(<Counter count={1} />, () =>
Scheduler.log('Sync effect'),
);
} else {
React.startTransition(() => {
ReactNoop.render(<Counter count={1} />, () =>
Scheduler.log('Sync effect'),
);
});
}
});

await waitFor(['Schedule update [0]', 'Count: 0']);

if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
expect(ReactNoop).toMatchRenderedOutput(
<span prop="Count: (empty)" />,
);
await waitFor(['Sync effect']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 0" />);

ReactNoop.flushPassiveEffects();
assertLog(['Schedule update [1]']);
await waitForAll(['Count: 1']);
} else {
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 0" />);
await waitFor([
'Count: 0',
'Sync effect',
'Schedule update [1]',
'Count: 1',
]);
}
expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 0" />);
await waitFor([
'Count: 0',
'Sync effect',
'Schedule update [1]',
'Count: 1',
]);

expect(ReactNoop).toMatchRenderedOutput(<span prop="Count: 1" />);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,61 +86,6 @@ describe('ReactInterleavedUpdates', () => {
expect(root).toMatchRenderedOutput('222');
});

// @gate forceConcurrentByDefaultForTesting
it('low priority update during an interleaved event is not processed during the current render', async () => {
// Same as previous test, but the interleaved update is lower priority than
// the in-progress render.
const updaters = [];

function Child() {
const [state, setState] = useState(0);
useEffect(() => {
updaters.push(setState);
}, []);
return <Text text={state} />;
}

function updateChildren(value) {
for (let i = 0; i < updaters.length; i++) {
const setState = updaters[i];
setState(value);
}
}

const root = ReactNoop.createRoot();

await act(async () => {
root.render(
<>
<Child />
<Child />
<Child />
</>,
);
});
assertLog([0, 0, 0]);
expect(root).toMatchRenderedOutput('000');

await act(async () => {
updateChildren(1);
// Partially render the children. Only the first one.
await waitFor([1]);

// In an interleaved event, schedule an update on each of the children.
// Including the two that haven't rendered yet.
startTransition(() => {
updateChildren(2);
});

// We should continue rendering without including the interleaved updates.
await waitForPaint([1, 1]);
expect(root).toMatchRenderedOutput('111');
});
// The interleaved updates flush in a separate render.
assertLog([2, 2, 2]);
expect(root).toMatchRenderedOutput('222');
});

it('regression for #24350: does not add to main update queue until interleaved update queue has been cleared', async () => {
let setStep;
function App() {
Expand Down
Loading

0 comments on commit e902c45

Please sign in to comment.