Skip to content

Commit

Permalink
[Bugfix] Pending state is always user-blocking (#17382)
Browse files Browse the repository at this point in the history
Fixes a bug where `isPending` is only set to `true` if `startTransition`
is called from inside an input event. That's usually the case, but
not always.

Now it works regardless of where you call it.
  • Loading branch information
acdlite committed Nov 15, 2019
1 parent 8e74a31 commit 2586303
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 54 deletions.
107 changes: 53 additions & 54 deletions packages/react-reconciler/src/ReactFiberHooks.js
Expand Up @@ -19,7 +19,6 @@ import type {HookEffectTag} from './ReactHookEffectTags';
import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';

import * as Scheduler from 'scheduler';
import ReactSharedInternals from 'shared/ReactSharedInternals';

import {NoWork} from './ReactFiberExpirationTime';
Expand Down Expand Up @@ -53,7 +52,12 @@ import getComponentName from 'shared/getComponentName';
import is from 'shared/objectIs';
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration';
import {
UserBlockingPriority,
NormalPriority,
runWithPriority,
getCurrentPriorityLevel,
} from './SchedulerWithReactIntegration';

const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

Expand Down Expand Up @@ -1135,15 +1139,13 @@ function mountDeferredValue<T>(
const [prevValue, setValue] = mountState(value);
mountEffect(
() => {
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
},
[value, config],
);
Expand All @@ -1157,65 +1159,62 @@ function updateDeferredValue<T>(
const [prevValue, setValue] = updateState(value);
updateEffect(
() => {
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setValue(value);
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
},
[value, config],
);
return prevValue;
}

function startTransition(setPending, config, callback) {
const priorityLevel = getCurrentPriorityLevel();
runWithPriority(
priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel,
() => {
setPending(true);
},
);
runWithPriority(
priorityLevel > NormalPriority ? NormalPriority : priorityLevel,
() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
},
);
}

function mountTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
const [isPending, setPending] = mountState(false);
const startTransition = mountCallback(
callback => {
setPending(true);
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
},
[config, isPending],
);
return [startTransition, isPending];
const start = mountCallback(startTransition.bind(null, setPending, config), [
setPending,
config,
]);
return [start, isPending];
}

function updateTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
const [isPending, setPending] = updateState(false);
const startTransition = updateCallback(
callback => {
setPending(true);
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
},
[config, isPending],
);
return [startTransition, isPending];
const start = updateCallback(startTransition.bind(null, setPending, config), [
setPending,
config,
]);
return [start, isPending];
}

function dispatchAction<S, A>(
Expand Down
@@ -0,0 +1,102 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
* @jest-environment node
*/

'use strict';

let ReactFeatureFlags;
let React;
let ReactNoop;
let Scheduler;
let Suspense;
let useState;
let useTransition;
let act;

describe('ReactHooksWithNoopRenderer', () => {
beforeEach(() => {
jest.resetModules();

ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.enableSchedulerTracing = true;
ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
useState = React.useState;
useTransition = React.useTransition;
Suspense = React.Suspense;
act = ReactNoop.act;
});

function Text(props) {
Scheduler.unstable_yieldValue(props.text);
return props.text;
}

function createAsyncText(text) {
let resolved = false;
let Component = function() {
if (!resolved) {
Scheduler.unstable_yieldValue('Suspend! [' + text + ']');
throw promise;
}
return <Text text={text} />;
};
let promise = new Promise(resolve => {
Component.resolve = function() {
resolved = true;
return resolve();
};
});
return Component;
}

it('isPending works even if called from outside an input event', async () => {
const Async = createAsyncText('Async');
let start;
function App() {
const [show, setShow] = useState(false);
const [startTransition, isPending] = useTransition();
start = () => startTransition(() => setShow(true));
return (
<Suspense fallback={<Text text="Loading..." />}>
{isPending ? <Text text="Pending..." /> : null}
{show ? <Async /> : <Text text="(empty)" />}
</Suspense>
);
}

const root = ReactNoop.createRoot();

await act(async () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['(empty)']);
expect(root).toMatchRenderedOutput('(empty)');

await act(async () => {
start();

expect(Scheduler).toFlushAndYield([
'Pending...',
'(empty)',
'Suspend! [Async]',
'Loading...',
]);

expect(root).toMatchRenderedOutput('Pending...(empty)');

await Async.resolve();
});
expect(Scheduler).toHaveYielded(['Async']);
expect(root).toMatchRenderedOutput('Async');
});
});

0 comments on commit 2586303

Please sign in to comment.