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

Migrate useDeferredValue and useTransition #17058

Merged
merged 11 commits into from
Oct 18, 2019
Merged

Migrate useDeferredValue and useTransition #17058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9ea8084
test
lunaruan Oct 10, 2019
3b44db5
spacing
lunaruan Oct 10, 2019
10c30ee
spacing
lunaruan Oct 10, 2019
2dd8b35
Merge branch 'use_suspense_deferred_val' of https://github.com/lunaru…
lunaruan Oct 10, 2019
0067615
renamed to useTransition and useDeferredValue
lunaruan Oct 10, 2019
ab07c5a
use enableStableConcurrentModeAPIs and updated warning
lunaruan Oct 10, 2019
7b7db2d
added a busyDelayMs/busyMinDurationMs test
lunaruan Oct 11, 2019
24e4bfa
Merge branch 'master' of https://github.com/facebook/react into use_s…
lunaruan Oct 16, 2019
4fdb6e0
updated according to comments
lunaruan Oct 18, 2019
c0ddda1
fixed ReactHooks test
lunaruan Oct 18, 2019
666e922
removed warnings
lunaruan Oct 18, 2019
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
27 changes: 26 additions & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import type {
ReactEventResponderListener,
} from 'shared/ReactTypes';
import type {Fiber} from 'react-reconciler/src/ReactFiber';
import type {Hook} from 'react-reconciler/src/ReactFiberHooks';
import type {Hook, TimeoutConfig} from 'react-reconciler/src/ReactFiberHooks';
import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks';
import type {SuspenseConfig} from 'react-reconciler/src/ReactFiberSuspenseConfig';

import ErrorStackParser from 'error-stack-parser';
import ReactSharedInternals from 'shared/ReactSharedInternals';
Expand Down Expand Up @@ -236,6 +237,28 @@ function useResponder(
};
}

function useTransition(
config: SuspenseConfig | null | void,
): [(() => void) => void, boolean] {
nextHook();
hookLog.push({
primitive: 'Transition',
stackError: new Error(),
value: config,
});
return [callback => {}, false];
}

function useDeferredValue<T>(value: T, config: TimeoutConfig | null | void): T {
nextHook();
hookLog.push({
primitive: 'DeferredValue',
stackError: new Error(),
value,
});
return value;
}

const Dispatcher: DispatcherType = {
readContext,
useCallback,
Expand All @@ -249,6 +272,8 @@ const Dispatcher: DispatcherType = {
useRef,
useState,
useResponder,
useTransition,
useDeferredValue,
};

// Inspect
Expand Down
24 changes: 22 additions & 2 deletions packages/react-dom/src/server/ReactPartialRendererHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
* @flow
*/

import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks';
import type {
Dispatcher as DispatcherType,
TimeoutConfig,
} from 'react-reconciler/src/ReactFiberHooks';
import type {ThreadID} from './ReactThreadIDAllocator';
import type {
ReactContext,
ReactEventResponderListener,
} from 'shared/ReactTypes';

import type {SuspenseConfig} from 'react-reconciler/src/ReactFiberSuspenseConfig';
import {validateContextBounds} from './ReactPartialRendererContext';

import invariant from 'shared/invariant';
Expand Down Expand Up @@ -457,6 +460,21 @@ function useResponder(responder, props): ReactEventResponderListener<any, any> {
};
}

function useDeferredValue<T>(value: T, config: TimeoutConfig | null | void): T {
resolveCurrentlyRenderingComponent();
return value;
}

function useTransition(
config: SuspenseConfig | null | void,
): [(callback: () => void) => void, boolean] {
resolveCurrentlyRenderingComponent();
const startTransition = callback => {
callback();
};
return [startTransition, false];
}

function noop(): void {}

export let currentThreadID: ThreadID = 0;
Expand All @@ -481,4 +499,6 @@ export const Dispatcher: DispatcherType = {
// Debugging effect
useDebugValue: noop,
useResponder,
useDeferredValue,
useTransition,
};
175 changes: 173 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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 @@ -54,7 +55,7 @@ import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration';

const {ReactCurrentDispatcher} = ReactSharedInternals;
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;

export type Dispatcher = {
readContext<T>(
Expand Down Expand Up @@ -92,6 +93,10 @@ export type Dispatcher = {
responder: ReactEventResponder<E, C>,
props: Object,
): ReactEventResponderListener<E, C>,
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T,
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean],
};

type Update<S, A> = {
Expand Down Expand Up @@ -123,7 +128,9 @@ export type HookType =
| 'useMemo'
| 'useImperativeHandle'
| 'useDebugValue'
| 'useResponder';
| 'useResponder'
| 'useDeferredValue'
| 'useTransition';

let didWarnAboutMismatchedHooksForComponent;
if (__DEV__) {
Expand Down Expand Up @@ -152,6 +159,10 @@ export type FunctionComponentUpdateQueue = {
lastEffect: Effect | null,
};

export type TimeoutConfig = {|
timeoutMs: number,
|};

type BasicStateAction<S> = (S => S) | S;

type Dispatch<A> = A => void;
Expand Down Expand Up @@ -1117,6 +1128,96 @@ function updateMemo<T>(
return nextValue;
}

function mountDeferredValue<T>(
value: T,
config: TimeoutConfig | void | null,
): 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;
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we not pass the TimeoutConfig to Scheduler.unstable_next in the mount case, when we do in the update case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch. unstable_next doesn't accept a second argument. It must have been a copypasta from withSuspenseConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it was accidental copy/paste. Deleted it! :)

},
[value, config],
);
return prevValue;
}

function updateDeferredValue<T>(
value: T,
config: TimeoutConfig | void | null,
): 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;
}
});
},
[value, config],
);
return prevValue;
}

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();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the callback run as a potentially separate commit, or does being inside Scheduler.next prevent non-explicit "unbatching"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The intention here is to:

  1. Set pending false at high priority
  2. Batch setting pending to false and doing the actual state change at lower priority

As a result, you'll see pending immediately turn true, but then it turn false together with the actual state change you wanted to do.

} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
},
[config, isPending],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we pass isPending as a dependency here?

);
return [startTransition, isPending];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only built-in hook producing a pair which returns a function as the first, instead of the second element. This will take some education.

Copy link
Contributor

Choose a reason for hiding this comment

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

That does seem odd. Should it be flipped?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The first argument is useful without the second but not the inverse.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn’t we nudge you to use the busy return value tho?

}

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];
}

function dispatchAction<S, A>(
fiber: Fiber,
queue: UpdateQueue<S, A>,
Expand Down Expand Up @@ -1272,6 +1373,8 @@ export const ContextOnlyDispatcher: Dispatcher = {
useState: throwInvalidHookError,
useDebugValue: throwInvalidHookError,
useResponder: throwInvalidHookError,
useDeferredValue: throwInvalidHookError,
useTransition: throwInvalidHookError,
};

const HooksDispatcherOnMount: Dispatcher = {
Expand All @@ -1288,6 +1391,8 @@ const HooksDispatcherOnMount: Dispatcher = {
useState: mountState,
useDebugValue: mountDebugValue,
useResponder: createResponderListener,
useDeferredValue: mountDeferredValue,
useTransition: mountTransition,
};

const HooksDispatcherOnUpdate: Dispatcher = {
Expand All @@ -1304,6 +1409,8 @@ const HooksDispatcherOnUpdate: Dispatcher = {
useState: updateState,
useDebugValue: updateDebugValue,
useResponder: createResponderListener,
useDeferredValue: updateDeferredValue,
useTransition: updateTransition,
};

let HooksDispatcherOnMountInDEV: Dispatcher | null = null;
Expand Down Expand Up @@ -1441,6 +1548,18 @@ if (__DEV__) {
mountHookTypesDev();
return createResponderListener(responder, props);
},
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T {
currentHookNameInDev = 'useDeferredValue';
mountHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
mountHookTypesDev();
return mountTransition(config);
},
};

HooksDispatcherOnMountWithHookTypesInDEV = {
Expand Down Expand Up @@ -1546,6 +1665,18 @@ if (__DEV__) {
updateHookTypesDev();
return createResponderListener(responder, props);
},
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T {
currentHookNameInDev = 'useDeferredValue';
updateHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
updateHookTypesDev();
return mountTransition(config);
},
};

HooksDispatcherOnUpdateInDEV = {
Expand Down Expand Up @@ -1651,6 +1782,18 @@ if (__DEV__) {
updateHookTypesDev();
return createResponderListener(responder, props);
},
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T {
currentHookNameInDev = 'useDeferredValue';
updateHookTypesDev();
return updateDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
updateHookTypesDev();
return updateTransition(config);
},
};

InvalidNestedHooksDispatcherOnMountInDEV = {
Expand Down Expand Up @@ -1768,6 +1911,20 @@ if (__DEV__) {
mountHookTypesDev();
return createResponderListener(responder, props);
},
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T {
currentHookNameInDev = 'useDeferredValue';
warnInvalidHookAccess();
mountHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
warnInvalidHookAccess();
mountHookTypesDev();
return mountTransition(config);
},
};

InvalidNestedHooksDispatcherOnUpdateInDEV = {
Expand Down Expand Up @@ -1885,5 +2042,19 @@ if (__DEV__) {
updateHookTypesDev();
return createResponderListener(responder, props);
},
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T {
currentHookNameInDev = 'useDeferredValue';
warnInvalidHookAccess();
updateHookTypesDev();
return updateDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
warnInvalidHookAccess();
updateHookTypesDev();
return updateTransition(config);
},
};
}
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2930,7 +2930,7 @@ function flushSuspensePriorityWarningInDEV() {
'update to provide immediate feedback, and another update that ' +
'triggers the bulk of the changes.' +
'\n\n' +
'Refer to the documentation for useSuspenseTransition to learn how ' +
'Refer to the documentation for useTransition to learn how ' +
'to implement this pattern.',
// TODO: Add link to React docs with more information, once it exists
componentNames.sort().join(', '),
Expand Down
Loading