Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete flushSuspenseFallbacksInTests flag #18596

Merged
merged 2 commits into from
Apr 14, 2020
Merged

Delete flushSuspenseFallbacksInTests flag #18596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2c2cf5
Move renderer `act` to work loop
acdlite Apr 13, 2020
3a12db4
Delete `flushSuspenseFallbacksInTests`
acdlite Apr 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

let React;
let ReactFeatureFlags;
let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
Expand All @@ -39,9 +38,6 @@ function initModules() {
// Reset warning cache.
jest.resetModuleRegistry();

ReactFeatureFlags = require('shared/ReactFeatureFlags');

ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
Expand Down Expand Up @@ -1281,13 +1277,26 @@ describe('ReactDOMServerHooks', () => {

// State update should trigger the ID to update, which changes the props
// of ChildWithID. This should cause ChildWithID to hydrate before Children
expect(Scheduler).toFlushAndYieldThrough([
'Child with ID',
'Child with ID',
'Child with ID',
'Child One',
'Child Two',
]);

expect(Scheduler).toFlushAndYieldThrough(
__DEV__
? [
'Child with ID',
// Fallbacks are immdiately committed in TestUtils version
// of act
// 'Child with ID',
// 'Child with ID',
'Child One',
'Child Two',
]
: [
'Child with ID',
'Child with ID',
'Child with ID',
'Child One',
'Child Two',
],
);

expect(child1Ref.current).toBe(null);
expect(childWithIDRef.current).toEqual(
Expand Down
189 changes: 3 additions & 186 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
} from './ReactFiberHostConfig';
import type {RendererInspectionConfig} from './ReactFiberHostConfig';
import {FundamentalComponent} from './ReactWorkTags';
import type {ReactNodeList, Thenable} from 'shared/ReactTypes';
import type {ReactNodeList} from 'shared/ReactTypes';
import type {ExpirationTime} from './ReactFiberExpirationTime.new';
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';

Expand Down Expand Up @@ -64,6 +64,7 @@ import {
warnIfNotScopedWithMatchingAct,
warnIfUnmockedScheduler,
IsThisRendererActing,
act,
} from './ReactFiberWorkLoop.new';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue.new';
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
Expand All @@ -85,11 +86,6 @@ import {
findHostInstancesForRefresh,
} from './ReactFiberHotReloading.new';

// used by isTestEnvironment builds
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';
// end isTestEnvironment imports

export {createPortal} from './ReactPortal';

type OpaqueRoot = FiberRoot;
Expand Down Expand Up @@ -308,6 +304,7 @@ export {
flushSync,
flushPassiveEffects,
IsThisRendererActing,
act,
};

export function getPublicRootInstance(
Expand Down Expand Up @@ -547,183 +544,3 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
getCurrentFiber: __DEV__ ? getCurrentFiberForDevTools : null,
});
}

const {IsSomeRendererActing} = ReactSharedInternals;
const isSchedulerMocked =
typeof Scheduler.unstable_flushAllWithoutAsserting === 'function';
const flushWork =
Scheduler.unstable_flushAllWithoutAsserting ||
function() {
let didFlushWork = false;
while (flushPassiveEffects()) {
didFlushWork = true;
}

return didFlushWork;
};

function flushWorkAndMicroTasks(onDone: (err: ?Error) => void) {
try {
flushWork();
enqueueTask(() => {
if (flushWork()) {
flushWorkAndMicroTasks(onDone);
} else {
onDone();
}
});
} catch (err) {
onDone(err);
}
}

// we track the 'depth' of the act() calls with this counter,
// so we can tell if any async act() calls try to run in parallel.

let actingUpdatesScopeDepth = 0;
let didWarnAboutUsingActInProd = false;

// eslint-disable-next-line no-inner-declarations
export function act(callback: () => Thenable<mixed>): Thenable<void> {
if (!__DEV__) {
if (didWarnAboutUsingActInProd === false) {
didWarnAboutUsingActInProd = true;
// eslint-disable-next-line react-internal/no-production-logging
console.error(
'act(...) is not supported in production builds of React, and might not behave as expected.',
);
}
}

const previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
actingUpdatesScopeDepth++;

const previousIsSomeRendererActing = IsSomeRendererActing.current;
const previousIsThisRendererActing = IsThisRendererActing.current;
IsSomeRendererActing.current = true;
IsThisRendererActing.current = true;

function onDone() {
actingUpdatesScopeDepth--;
IsSomeRendererActing.current = previousIsSomeRendererActing;
IsThisRendererActing.current = previousIsThisRendererActing;
if (__DEV__) {
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
console.error(
'You seem to have overlapping act() calls, this is not supported. ' +
'Be sure to await previous act() calls before making a new one. ',
);
}
}
}

let result;
try {
result = batchedUpdates(callback);
} catch (error) {
// on sync errors, we still want to 'cleanup' and decrement actingUpdatesScopeDepth
onDone();
throw error;
}

if (
result !== null &&
typeof result === 'object' &&
typeof result.then === 'function'
) {
// setup a boolean that gets set to true only
// once this act() call is await-ed
let called = false;
if (__DEV__) {
if (typeof Promise !== 'undefined') {
//eslint-disable-next-line no-undef
Promise.resolve()
.then(() => {})
.then(() => {
if (called === false) {
console.error(
'You called act(async () => ...) without await. ' +
'This could lead to unexpected testing behaviour, interleaving multiple act ' +
'calls and mixing their scopes. You should - await act(async () => ...);',
);
}
});
}
}

// in the async case, the returned thenable runs the callback, flushes
// effects and microtasks in a loop until flushPassiveEffects() === false,
// and cleans up
return {
then(resolve, reject) {
called = true;
result.then(
() => {
if (
actingUpdatesScopeDepth > 1 ||
(isSchedulerMocked === true &&
previousIsSomeRendererActing === true)
) {
onDone();
resolve();
return;
}
// we're about to exit the act() scope,
// now's the time to flush tasks/effects
flushWorkAndMicroTasks((err: ?Error) => {
onDone();
if (err) {
reject(err);
} else {
resolve();
}
});
},
err => {
onDone();
reject(err);
},
);
},
};
} else {
if (__DEV__) {
if (result !== undefined) {
console.error(
'The callback passed to act(...) function ' +
'must return undefined, or a Promise. You returned %s',
result,
);
}
}

// flush effects until none remain, and cleanup
try {
if (
actingUpdatesScopeDepth === 1 &&
(isSchedulerMocked === false || previousIsSomeRendererActing === false)
) {
// we're about to exit the act() scope,
// now's the time to flush effects
flushWork();
}
onDone();
} catch (err) {
onDone();
throw err;
}

// in the sync case, the returned thenable only warns *if* await-ed
return {
then(resolve) {
if (__DEV__) {
console.error(
'Do not await the result of calling act(...) with sync logic, it is not a Promise.',
);
}
resolve();
},
};
}
}
Loading