Remove forceConcurrentByDefaultForTesting flag (#30436)

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.
This commit is contained in:
Jack Pope
2024-07-24 10:17:33 -04:00
committed by GitHub
parent 5b2c7e62c8
commit 236f3951c7
20 changed files with 57 additions and 363 deletions
+1 -61
View File
@@ -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;
@@ -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', () => {
@@ -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');
});
@@ -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.
});
-6
View File
@@ -31,7 +31,6 @@ import {
enableProfilerTimer,
enableScopeAPI,
enableLegacyHidden,
forceConcurrentByDefaultForTesting,
allowConcurrentByDefault,
enableTransitionTracing,
enableDebugTracing,
@@ -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
@@ -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);
ReactNoop.render(<Text text="Step 1" />);
React.startTransition(() => {
ReactNoop.render(<Text text="Step 2" />);
});
await waitFor(['Step 1']);
// 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']);
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 () => {
@@ -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);
// 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');
// 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']);
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 () => {
@@ -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" />);
});
@@ -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() {
@@ -378,74 +378,6 @@ describe('ReactSuspense', () => {
expect(container.textContent).toEqual('AB');
});
// @gate !disableLegacyMode && forceConcurrentByDefaultForTesting
it(
'interrupts current render when something suspends with a ' +
"delay and we've already skipped over a lower priority update in " +
'a parent',
async () => {
const root = ReactDOMClient.createRoot(container);
function interrupt() {
// React has a heuristic to batch all updates that occur within the same
// event. This is a trick to circumvent that heuristic.
ReactDOM.render('whatever', document.createElement('div'));
}
function App({shouldSuspend, step}) {
return (
<>
<Text text={`A${step}`} />
<Suspense fallback={<Text text="Loading..." />}>
{shouldSuspend ? <AsyncText text="Async" /> : null}
</Suspense>
<Text text={`B${step}`} />
<Text text={`C${step}`} />
</>
);
}
root.render(<App shouldSuspend={false} step={0} />);
await waitForAll(['A0', 'B0', 'C0']);
expect(container.textContent).toEqual('A0B0C0');
// This update will suspend.
root.render(<App shouldSuspend={true} step={1} />);
// Do a bit of work
await waitFor(['A1']);
// Schedule another update. This will have lower priority because it's
// a transition.
React.startTransition(() => {
root.render(<App shouldSuspend={false} step={2} />);
});
// Interrupt to trigger a restart.
interrupt();
await waitFor([
// Should have restarted the first update, because of the interruption
'A1',
'Suspend! [Async]',
'Loading...',
'B1',
]);
// Should not have committed loading state
expect(container.textContent).toEqual('A0B0C0');
// After suspending, should abort the first update and switch to the
// second update. So, C1 should not appear in the log.
// TODO: This should work even if React does not yield to the main
// thread. Should use same mechanism as selective hydration to interrupt
// the render before the end of the current slice of work.
await waitForAll(['A2', 'B2', 'C2']);
expect(container.textContent).toEqual('A2B2C2');
},
);
// @gate !disableLegacyMode
it('mounts a lazy class component in non-concurrent mode (legacy)', async () => {
class Class extends React.Component {
@@ -2234,27 +2234,14 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await waitForAll(['Foo', 'A']);
expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
React.startTransition(() => {
ReactNoop.render(<Foo showB={true} />);
} else {
React.startTransition(() => {
ReactNoop.render(<Foo showB={true} />);
});
}
});
await waitForAll(['Foo', 'A', 'Suspend! [B]', 'Loading B...']);
if (gate(flags => flags.forceConcurrentByDefaultForTesting)) {
expect(ReactNoop).toMatchRenderedOutput(
<>
<span prop="A" />
<span prop="Loading B..." />
</>,
);
} else {
// Transitions never fall back.
expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
}
// Transitions never fall back.
expect(ReactNoop).toMatchRenderedOutput(<span prop="A" />);
});
// @gate enableLegacyCache
@@ -1656,7 +1656,6 @@ describe('ReactUse', () => {
expect(root).toMatchRenderedOutput('C');
});
// @gate !forceConcurrentByDefaultForTesting
it('an async component outside of a Suspense boundary crashes with an error (resolves in microtask)', async () => {
class ErrorBoundary extends React.Component {
state = {error: null};
@@ -1708,7 +1707,6 @@ describe('ReactUse', () => {
);
});
// @gate !forceConcurrentByDefaultForTesting
it('an async component outside of a Suspense boundary crashes with an error (resolves in macrotask)', async () => {
class ErrorBoundary extends React.Component {
state = {error: null};
@@ -170,17 +170,6 @@ describe(`onRender`, () => {
'read current time',
'read current time',
]);
} else if (gate(flags => !flags.allowConcurrentByDefault)) {
assertLog([
'read current time',
'read current time',
'read current time',
'read current time',
'read current time',
'read current time',
'read current time',
// TODO: why is there one less in this case?
]);
} else {
assertLog([
'read current time',
@@ -190,7 +179,6 @@ describe(`onRender`, () => {
'read current time',
'read current time',
'read current time',
'read current time',
]);
}
});
-3
View File
@@ -217,9 +217,6 @@ export const enableUseDeferredValueInitialArg = true;
// when we plan to enable them.
// -----------------------------------------------------------------------------
// Enables time slicing for updates that aren't wrapped in startTransition.
export const forceConcurrentByDefaultForTesting = false;
// Adds an opt-in to time slicing for updates that aren't wrapped in startTransition.
export const allowConcurrentByDefault = false;
@@ -85,7 +85,6 @@ export const enableUseDeferredValueInitialArg = true;
export const enableUseEffectEventHook = false;
export const enableUseMemoCacheHook = true;
export const favorSafetyOverHydrationPerf = true;
export const forceConcurrentByDefaultForTesting = false;
export const renameElementSymbol = false;
export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
@@ -76,7 +76,6 @@ export const enableUseDeferredValueInitialArg = true;
export const enableUseEffectEventHook = false;
export const enableUseMemoCacheHook = true;
export const favorSafetyOverHydrationPerf = true;
export const forceConcurrentByDefaultForTesting = false;
export const passChildrenWhenCloningPersistedNodes = false;
export const renameElementSymbol = true;
export const retryLaneExpirationMs = 5000;
@@ -53,7 +53,6 @@ export const transitionLaneExpirationMs = 5000;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const forceConcurrentByDefaultForTesting = false;
export const allowConcurrentByDefault = false;
export const consoleManagedByDevToolsDuringStrictMode = false;
@@ -72,7 +72,6 @@ export const enableUseDeferredValueInitialArg = true;
export const enableUseEffectEventHook = false;
export const enableUseMemoCacheHook = true;
export const favorSafetyOverHydrationPerf = true;
export const forceConcurrentByDefaultForTesting = false;
export const passChildrenWhenCloningPersistedNodes = false;
export const renameElementSymbol = false;
export const retryLaneExpirationMs = 5000;
@@ -56,7 +56,6 @@ export const transitionLaneExpirationMs = 5000;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const forceConcurrentByDefaultForTesting = false;
export const allowConcurrentByDefault = true;
export const consoleManagedByDevToolsDuringStrictMode = false;
@@ -29,7 +29,6 @@ export const enableRetryLaneExpiration = __VARIANT__;
export const enableTransitionTracing = __VARIANT__;
export const enableUseDeferredValueInitialArg = __VARIANT__;
export const favorSafetyOverHydrationPerf = __VARIANT__;
export const forceConcurrentByDefaultForTesting = __VARIANT__;
export const renameElementSymbol = __VARIANT__;
export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
@@ -102,8 +102,6 @@ export const consoleManagedByDevToolsDuringStrictMode = true;
export const enableFizzExternalRuntime = true;
export const forceConcurrentByDefaultForTesting = false;
export const passChildrenWhenCloningPersistedNodes = false;
export const enableAsyncDebugInfo = false;
-3
View File
@@ -8,9 +8,6 @@ jest.mock('shared/ReactFeatureFlags', () => {
);
const actual = jest.requireActual('shared/forks/ReactFeatureFlags.www');
// This flag is only used by tests, it should never be set elsewhere.
actual.forceConcurrentByDefaultForTesting = !__VARIANT__;
// Flags that aren't currently used, but we still want to force variants to keep the
// code live.
actual.disableInputAttributeSyncing = __VARIANT__;