Skip to content

Commit

Permalink
Remove maxDuration from tests (#15272)
Browse files Browse the repository at this point in the history
We instead assume a 150ms duration.
  • Loading branch information
sebmarkbage authored Apr 2, 2019
1 parent 9307932 commit 4c75881
Show file tree
Hide file tree
Showing 12 changed files with 141 additions and 188 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -720,16 +720,14 @@ describe('ReactDOMFiberAsync', () => {

let root = ReactDOM.unstable_createRoot(container);
root.render(
<React.Suspense maxDuration={1000} fallback={'Loading'}>
Initial
</React.Suspense>,
<React.Suspense fallback={'Loading'}>Initial</React.Suspense>,
);

Scheduler.flushAll();
expect(container.textContent).toBe('Initial');

root.render(
<React.Suspense maxDuration={1000} fallback={'Loading'}>
<React.Suspense fallback={'Loading'}>
<Suspend />
</React.Suspense>,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ describe('ReactDOMServerPartialHydration', () => {
<div>
<Suspense fallback="Loading...">
<span ref={ref} className={className}>
<Suspense maxDuration={200}>
<Suspense>
<Child text={text} />
</Suspense>
</span>
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('ReactDOMServerPartialHydration', () => {
expect(ref.current).toBe(span);
});

it('replaces the fallback within the maxDuration if there is a nested suspense', async () => {
it('replaces the fallback within the suspended time if there is a nested suspense', async () => {
let suspend = false;
let promise = new Promise(resolvePromise => {});
let ref = React.createRef();
Expand All @@ -724,7 +724,7 @@ describe('ReactDOMServerPartialHydration', () => {
function App() {
return (
<div>
<Suspense fallback="Loading..." maxDuration={100}>
<Suspense fallback="Loading...">
<span ref={ref}>
<Child />
</span>
Expand All @@ -751,7 +751,7 @@ describe('ReactDOMServerPartialHydration', () => {
let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
root.render(<App />);
Scheduler.flushAll();
// This will have exceeded the maxDuration so we should timeout.
// This will have exceeded the suspended time so we should timeout.
jest.advanceTimersByTime(500);
// The boundary should longer be suspended for the middle content
// even though the inner boundary is still suspended.
Expand All @@ -762,7 +762,7 @@ describe('ReactDOMServerPartialHydration', () => {
expect(ref.current).toBe(span);
});

it('replaces the fallback within the maxDuration if there is a nested suspense in a nested suspense', async () => {
it('replaces the fallback within the suspended time if there is a nested suspense in a nested suspense', async () => {
let suspend = false;
let promise = new Promise(resolvePromise => {});
let ref = React.createRef();
Expand All @@ -784,7 +784,7 @@ describe('ReactDOMServerPartialHydration', () => {
return (
<div>
<Suspense fallback="Another layer">
<Suspense fallback="Loading..." maxDuration={100}>
<Suspense fallback="Loading...">
<span ref={ref}>
<Child />
</span>
Expand Down Expand Up @@ -812,7 +812,7 @@ describe('ReactDOMServerPartialHydration', () => {
let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
root.render(<App />);
Scheduler.flushAll();
// This will have exceeded the maxDuration so we should timeout.
// This will have exceeded the suspended time so we should timeout.
jest.advanceTimersByTime(500);
// The boundary should longer be suspended for the middle content
// even though the inner boundary is still suspended.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ describe('ReactDOMSuspensePlaceholder', () => {
];
function App() {
return (
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<div ref={divs[0]}>
<Text text="A" />
</div>
<div ref={divs[1]}>
<AsyncText ms={1000} text="B" />
<AsyncText ms={500} text="B" />
</div>
<div style={{display: 'block'}} ref={divs[2]}>
<Text text="C" />
Expand All @@ -92,7 +92,7 @@ describe('ReactDOMSuspensePlaceholder', () => {
expect(divs[1].current.style.display).toEqual('none');
expect(divs[2].current.style.display).toEqual('none');

await advanceTimers(1000);
await advanceTimers(500);

expect(divs[0].current.style.display).toEqual('');
expect(divs[1].current.style.display).toEqual('');
Expand All @@ -103,17 +103,17 @@ describe('ReactDOMSuspensePlaceholder', () => {
it('hides and unhides timed out text nodes', async () => {
function App() {
return (
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Text text="A" />
<AsyncText ms={1000} text="B" />
<AsyncText ms={500} text="B" />
<Text text="C" />
</Suspense>
);
}
ReactDOM.render(<App />, container);
expect(container.textContent).toEqual('Loading...');

await advanceTimers(1000);
await advanceTimers(500);

expect(container.textContent).toEqual('ABC');
});
Expand All @@ -137,10 +137,10 @@ describe('ReactDOMSuspensePlaceholder', () => {

function App() {
return (
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Sibling>Sibling</Sibling>
<span>
<AsyncText ms={1000} text="Async" />
<AsyncText ms={500} text="Async" />
</span>
</Suspense>
);
Expand All @@ -158,7 +158,7 @@ describe('ReactDOMSuspensePlaceholder', () => {
'<span style="display: none;">Sibling</span><span style="display: none;"></span>Loading...',
);

await advanceTimers(1000);
await advanceTimers(500);

expect(container.innerHTML).toEqual(
'<span style="display: inline;">Sibling</span><span style="">Async</span>',
Expand Down
15 changes: 15 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ let didWarnAboutContextTypeOnFunctionComponent;
let didWarnAboutGetDerivedStateOnFunctionComponent;
let didWarnAboutFunctionRefs;
export let didWarnAboutReassigningProps;
let didWarnAboutMaxDuration;

if (__DEV__) {
didWarnAboutBadClass = {};
Expand All @@ -165,6 +166,7 @@ if (__DEV__) {
didWarnAboutGetDerivedStateOnFunctionComponent = {};
didWarnAboutFunctionRefs = {};
didWarnAboutReassigningProps = false;
didWarnAboutMaxDuration = false;
}

export function reconcileChildren(
Expand Down Expand Up @@ -1409,6 +1411,19 @@ function updateSuspenseComponent(
workInProgress.effectTag &= ~DidCapture;
}

if (__DEV__) {
if ('maxDuration' in nextProps) {
if (!didWarnAboutMaxDuration) {
didWarnAboutMaxDuration = true;
warning(
false,
'maxDuration has been removed from React. ' +
'Remove the maxDuration prop.',
);
}
}
}

// This next part is a bit confusing. If the children timeout, we switch to
// showing the fallback children in place of the "primary" children.
// However, we don't want to delete the primary children because then their
Expand Down
16 changes: 6 additions & 10 deletions packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,12 @@ function throwException(
break;
}
}
let timeoutPropMs = workInProgress.pendingProps.maxDuration;
if (typeof timeoutPropMs === 'number') {
if (timeoutPropMs <= 0) {
earliestTimeoutMs = 0;
} else if (
earliestTimeoutMs === -1 ||
timeoutPropMs < earliestTimeoutMs
) {
earliestTimeoutMs = timeoutPropMs;
}
const defaultSuspenseTimeout = 150;
if (
earliestTimeoutMs === -1 ||
defaultSuspenseTimeout < earliestTimeoutMs
) {
earliestTimeoutMs = defaultSuspenseTimeout;
}
}
// If there is a DehydratedSuspenseComponent we don't have to do anything because
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ describe('ReactLazy', () => {
return <Text text="Bar" />;
}

const promiseForFoo = delay(1000).then(() => fakeImport(Foo));
const promiseForBar = delay(2000).then(() => fakeImport(Bar));
const promiseForFoo = delay(100).then(() => fakeImport(Foo));
const promiseForBar = delay(500).then(() => fakeImport(Bar));

const LazyFoo = lazy(() => promiseForFoo);
const LazyBar = lazy(() => promiseForBar);
Expand All @@ -138,13 +138,13 @@ describe('ReactLazy', () => {
expect(Scheduler).toFlushAndYield(['Loading...']);
expect(root).toMatchRenderedOutput(null);

jest.advanceTimersByTime(1000);
jest.advanceTimersByTime(100);
await promiseForFoo;

expect(Scheduler).toFlushAndYield(['Foo', 'Loading...']);
expect(root).toMatchRenderedOutput(null);

jest.advanceTimersByTime(1000);
jest.advanceTimersByTime(500);
await promiseForBar;

expect(Scheduler).toFlushAndYield(['Foo', 'Bar']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ describe('ReactSuspense', () => {
// Render two sibling Suspense components
const root = ReactTestRenderer.create(
<React.Fragment>
<Suspense maxDuration={1000} fallback={<Text text="Loading A..." />}>
<Suspense fallback={<Text text="Loading A..." />}>
<AsyncText text="A" ms={5000} />
</Suspense>
<Suspense maxDuration={3000} fallback={<Text text="Loading B..." />}>
<Suspense fallback={<Text text="Loading B..." />}>
<AsyncText text="B" ms={6000} />
</Suspense>
</React.Fragment>,
Expand Down Expand Up @@ -211,7 +211,7 @@ describe('ReactSuspense', () => {
}

const root = ReactTestRenderer.create(
<Suspense maxDuration={1000} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Async />
<Text text="Sibling" />
</Suspense>,
Expand Down Expand Up @@ -272,7 +272,7 @@ describe('ReactSuspense', () => {
it('only captures if `fallback` is defined', () => {
const root = ReactTestRenderer.create(
<Suspense fallback={<Text text="Loading..." />}>
<Suspense maxDuration={100}>
<Suspense>
<AsyncText text="Hi" ms={5000} />
</Suspense>
</Suspense>,
Expand Down Expand Up @@ -368,9 +368,7 @@ describe('ReactSuspense', () => {

function App() {
return (
<Suspense
maxDuration={1000}
fallback={<TextWithLifecycle text="Loading..." />}>
<Suspense fallback={<TextWithLifecycle text="Loading..." />}>
<TextWithLifecycle text="A" />
<AsyncTextWithLifecycle ms={100} text="B" ref={instance} />
<TextWithLifecycle text="C" />
Expand Down Expand Up @@ -631,7 +629,7 @@ describe('ReactSuspense', () => {

function App(props) {
return (
<Suspense maxDuration={10} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Stateful />
</Suspense>
);
Expand Down Expand Up @@ -681,7 +679,7 @@ describe('ReactSuspense', () => {

function App(props) {
return (
<Suspense maxDuration={10} fallback={<ShouldMountOnce />}>
<Suspense fallback={<ShouldMountOnce />}>
<AsyncText ms={1000} text="Child 1" />
<AsyncText ms={2000} text="Child 2" />
<AsyncText ms={3000} text="Child 3" />
Expand Down Expand Up @@ -726,7 +724,7 @@ describe('ReactSuspense', () => {
it('does not get stuck with fallback in concurrent mode for a large delay', () => {
function App(props) {
return (
<Suspense maxDuration={10} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText ms={1000} text="Child 1" />
<AsyncText ms={7000} text="Child 2" />
</Suspense>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,6 @@ describe('ReactSuspenseFuzz', () => {
remainingElements--;
const children = createRandomChildren(3);

const maxDuration = pickRandomWeighted(rand, [
{value: undefined, weight: 1},
{value: rand.intBetween(0, 5000), weight: 1},
]);

const fallbackType = pickRandomWeighted(rand, [
{value: 'none', weight: 1},
{value: 'normal', weight: 1},
Expand All @@ -290,11 +285,7 @@ describe('ReactSuspenseFuzz', () => {
);
}

return React.createElement(
Suspense,
{maxDuration, fallback},
...children,
);
return React.createElement(Suspense, {fallback}, ...children);
}
case 'return':
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('ReactSuspensePlaceholder', () => {

function App(props) {
return (
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<HiddenText text="A" />
<span>
<AsyncText ms={1000} text={props.middleText} />
Expand Down Expand Up @@ -176,7 +176,7 @@ describe('ReactSuspensePlaceholder', () => {
it('times out text nodes', async () => {
function App(props) {
return (
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Text text="A" />
<AsyncText ms={1000} text={props.middleText} />
<Text text="C" />
Expand Down Expand Up @@ -225,7 +225,7 @@ describe('ReactSuspensePlaceholder', () => {
// uppercase is a special type that causes React Noop to render child
// text nodes as uppercase.
<uppercase>
<Suspense maxDuration={500} fallback={<Text text="Loading..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<Text text="a" />
<AsyncText ms={1000} text={props.middleText} />
<Text text="c" />
Expand Down Expand Up @@ -293,7 +293,7 @@ describe('ReactSuspensePlaceholder', () => {
Scheduler.yieldValue('App');
return (
<Profiler id="root" onRender={onRender}>
<Suspense maxDuration={500} fallback={<Fallback />}>
<Suspense fallback={<Fallback />}>
{shouldSuspend && <Suspending />}
<Text fakeRenderDuration={textRenderDuration} text={text} />
</Suspense>
Expand Down
Loading

0 comments on commit 4c75881

Please sign in to comment.