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

Jump to bottom

Remove maxDuration #15272

Merged
merged 1 commit into from
Apr 2, 2019
Merged

Remove maxDuration #15272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js
View file
Open in desktop
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
14 changes: 7 additions & 7 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
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
18 changes: 9 additions & 9 deletions packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ let didWarnAboutContextTypeOnFunctionComponent;
let didWarnAboutGetDerivedStateOnFunctionComponent;
let didWarnAboutFunctionRefs;
export let didWarnAboutReassigningProps;
let didWarnAboutMaxDuration;

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

export function reconcileChildren(
Expand Down Expand Up @@ -1408,6 +1410,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a global const(config), why not lift it to the top?

if (
earliestTimeoutMs === -1 ||
defaultSuspenseTimeout < earliestTimeoutMs
) {
earliestTimeoutMs = defaultSuspenseTimeout;
}
}
// If there is a DehydratedSuspenseComponent we don't have to do anything because
Expand Down
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
View file
Open in desktop
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
18 changes: 8 additions & 10 deletions packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
View file
Open in desktop
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
11 changes: 1 addition & 10 deletions packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js
View file
Open in desktop
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