From ba6477aa3cc0b189f4c9b805bf20edb2a2d94e91 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 29 Jan 2019 17:39:24 -0800 Subject: [PATCH] Improve Reducer Hook's lazy init API (#14723) * Improve Reducer Hook's lazy init API * Use generic type for initilizer input Still requires an `any` cast in the case where `init` function is not provided. --- .../react-debug-tools/src/ReactDebugHooks.js | 13 ++-- ...DOMServerIntegrationHooks-test.internal.js | 4 +- .../src/server/ReactPartialRendererHooks.js | 19 +++--- .../react-reconciler/src/ReactFiberHooks.js | 60 +++++++++---------- .../src/__tests__/ReactHooks-test.internal.js | 18 +++--- ...eactHooksWithNoopRenderer-test.internal.js | 19 +++--- .../src/ReactShallowRenderer.js | 19 +++--- .../ReactShallowRendererHooks-test.js | 26 +++----- packages/react/src/ReactHooks.js | 8 +-- 9 files changed, 93 insertions(+), 93 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 426d20a1328a6..63d53b8508494 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -115,13 +115,18 @@ function useState( return [state, (action: BasicStateAction) => {}]; } -function useReducer( +function useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { let hook = nextHook(); - let state = hook !== null ? hook.memoizedState : initialState; + let state; + if (hook !== null) { + state = hook.memoizedState; + } else { + state = init !== undefined ? init(initialArg) : ((initialArg: any): S); + } hookLog.push({ primitive: 'Reducer', stackError: new Error(), diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index 5fa5c14f49ad9..9885cc0e74bef 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -208,12 +208,12 @@ describe('ReactDOMServerHooks', () => { expect(domNode.textContent).toEqual('0'); }); - itRenders('lazy initialization with initialAction', async render => { + itRenders('lazy initialization', async render => { function reducer(state, action) { return action === 'increment' ? state + 1 : state; } function Counter() { - let [count] = useReducer(reducer, 0, 'increment'); + let [count] = useReducer(reducer, 0, c => c + 1); yieldValue('Render: ' + count); return ; } diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js index c462487e380ff..82c61626ef088 100644 --- a/packages/react-dom/src/server/ReactPartialRendererHooks.js +++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js @@ -252,10 +252,10 @@ export function useState( ); } -export function useReducer( +export function useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { if (__DEV__) { if (reducer !== basicStateReducer) { @@ -301,13 +301,16 @@ export function useReducer( if (__DEV__) { isInHookUserCodeInDev = true; } + let initialState; if (reducer === basicStateReducer) { // Special case for `useState`. - if (typeof initialState === 'function') { - initialState = initialState(); - } - } else if (initialAction !== undefined && initialAction !== null) { - initialState = reducer(initialState, initialAction); + initialState = + typeof initialArg === 'function' + ? ((initialArg: any): () => S)() + : ((initialArg: any): S); + } else { + initialState = + init !== undefined ? init(initialArg) : ((initialArg: any): S); } if (__DEV__) { isInHookUserCodeInDev = false; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 2955a0e55e82a..21d55478870cc 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -49,10 +49,10 @@ export type Dispatcher = { observedBits: void | number | boolean, ): T, useState(initialState: (() => S) | S): [S, Dispatch>], - useReducer( + useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: (I) => S, ): [S, Dispatch], useContext( context: ReactContext, @@ -591,16 +591,17 @@ function updateContext( return readContext(context, observedBits); } -function mountReducer( +function mountReducer( reducer: (S, A) => S, - initialState: void | S, - initialAction: void | null | A, + initialArg: I, + init?: I => S, ): [S, Dispatch] { const hook = mountWorkInProgressHook(); - // TODO: Lazy init API will change before release. - if (initialAction !== undefined && initialAction !== null) { - // $FlowFixMe - Must express with overloading. - initialState = reducer(initialState, initialAction); + let initialState; + if (init !== undefined) { + initialState = init(initialArg); + } else { + initialState = ((initialArg: any): S); } hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { @@ -618,10 +619,10 @@ function mountReducer( return [hook.memoizedState, dispatch]; } -function updateReducer( +function updateReducer( reducer: (S, A) => S, - initialState: void | S, - initialAction: void | null | A, + initialArg: I, + init?: I => S, ): [S, Dispatch] { const hook = updateWorkInProgressHook(); const queue = hook.queue; @@ -755,7 +756,6 @@ function mountState( initialState: (() => S) | S, ): [S, Dispatch>] { const hook = mountWorkInProgressHook(); - // TODO: Lazy init API will change before release. if (typeof initialState === 'function') { initialState = initialState(); } @@ -1282,16 +1282,16 @@ if (__DEV__) { ReactCurrentDispatcher.current = prevDispatcher; } }, - useReducer( + useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { - return mountReducer(reducer, initialState, initialAction); + return mountReducer(reducer, initialArg, init); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1366,16 +1366,16 @@ if (__DEV__) { ReactCurrentDispatcher.current = prevDispatcher; } }, - useReducer( + useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateReducer(reducer, initialState, initialAction); + return updateReducer(reducer, initialArg, init); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1457,17 +1457,17 @@ if (__DEV__) { ReactCurrentDispatcher.current = prevDispatcher; } }, - useReducer( + useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { - return mountReducer(reducer, initialState, initialAction); + return mountReducer(reducer, initialArg, init); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1552,17 +1552,17 @@ if (__DEV__) { ReactCurrentDispatcher.current = prevDispatcher; } }, - useReducer( + useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateReducer(reducer, initialState, initialAction); + return updateReducer(reducer, initialArg, init); } finally { ReactCurrentDispatcher.current = prevDispatcher; } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 0a37db0d37b89..ff39ddee80246 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -752,19 +752,19 @@ describe('ReactHooks', () => { const ThemeContext = createContext('light'); function App() { - useReducer( - () => { - ReactCurrentDispatcher.current.readContext(ThemeContext); - }, - null, - {}, - ); + const [state, dispatch] = useReducer((s, action) => { + ReactCurrentDispatcher.current.readContext(ThemeContext); + return action; + }, 0); + if (state === 0) { + dispatch(1); + } return null; } - expect(() => ReactTestRenderer.create()).toWarnDev( + expect(() => ReactTestRenderer.create()).toWarnDev([ 'Context can only be read while React is rendering', - ); + ]); }); // Edge case. diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index 363ef3bd42393..2f74fdc9c885b 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -524,14 +524,12 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: -2')]); }); - it('accepts an initial action', () => { + it('lazy init', () => { const INCREMENT = 'INCREMENT'; const DECREMENT = 'DECREMENT'; function reducer(state, action) { switch (action) { - case 'INITIALIZE': - return 10; case 'INCREMENT': return state + 1; case 'DECREMENT': @@ -541,27 +539,28 @@ describe('ReactHooksWithNoopRenderer', () => { } } - const initialAction = 'INITIALIZE'; - function Counter(props, ref) { - const [count, dispatch] = useReducer(reducer, 0, initialAction); + const [count, dispatch] = useReducer(reducer, props, p => { + ReactNoop.yield('Init'); + return p.initialCount; + }); useImperativeHandle(ref, () => ({dispatch})); return ; } Counter = forwardRef(Counter); const counter = React.createRef(null); - ReactNoop.render(); - ReactNoop.flush(); + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Init', 'Count: 10']); expect(ReactNoop.getChildren()).toEqual([span('Count: 10')]); counter.current.dispatch(INCREMENT); - ReactNoop.flush(); + expect(ReactNoop.flush()).toEqual(['Count: 11']); expect(ReactNoop.getChildren()).toEqual([span('Count: 11')]); counter.current.dispatch(DECREMENT); counter.current.dispatch(DECREMENT); counter.current.dispatch(DECREMENT); - ReactNoop.flush(); + expect(ReactNoop.flush()).toEqual(['Count: 8']); expect(ReactNoop.getChildren()).toEqual([span('Count: 8')]); }); diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index ee8b68b0f857e..0701854452f2c 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -223,10 +223,10 @@ class ReactShallowRenderer { } _createDispatcher(): DispatcherType { - const useReducer = ( + const useReducer = ( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ): [S, Dispatch] => { this._validateCurrentlyRenderingComponent(); this._createWorkInProgressHook(); @@ -259,13 +259,16 @@ class ReactShallowRenderer { } return [workInProgressHook.memoizedState, dispatch]; } else { + let initialState; if (reducer === basicStateReducer) { // Special case for `useState`. - if (typeof initialState === 'function') { - initialState = initialState(); - } - } else if (initialAction !== undefined && initialAction !== null) { - initialState = reducer(initialState, initialAction); + initialState = + typeof initialArg === 'function' + ? ((initialArg: any): () => S)() + : ((initialArg: any): S); + } else { + initialState = + init !== undefined ? init(initialArg) : ((initialArg: any): S); } workInProgressHook.memoizedState = initialState; const queue: UpdateQueue = (workInProgressHook.queue = { diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js index 60c79a12d702b..da352a971e160 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRendererHooks-test.js @@ -91,23 +91,19 @@ describe('ReactShallowRenderer with hooks', () => { }); it('should work with useReducer', () => { - const initialState = {count: 0}; - function reducer(state, action) { switch (action.type) { - case 'reset': - return initialState; case 'increment': return {count: state.count + 1}; case 'decrement': return {count: state.count - 1}; - default: - return state; } } - function SomeComponent({initialCount}) { - const [state] = React.useReducer(reducer, {count: initialCount}); + function SomeComponent(props) { + const [state] = React.useReducer(reducer, props, p => ({ + count: p.initialCount, + })); return (
@@ -141,25 +137,19 @@ describe('ReactShallowRenderer with hooks', () => { }); it('should work with a dispatched state change for a useReducer', () => { - const initialState = {count: 0}; - function reducer(state, action) { switch (action.type) { - case 'reset': - return initialState; case 'increment': return {count: state.count + 1}; case 'decrement': return {count: state.count - 1}; - default: - return state; } } - function SomeComponent({initialCount}) { - const [state, dispatch] = React.useReducer(reducer, { - count: initialCount, - }); + function SomeComponent(props) { + const [state, dispatch] = React.useReducer(reducer, props, p => ({ + count: p.initialCount, + })); if (state.count === 0) { dispatch({type: 'increment'}); diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 37f85cd5c3537..07098fc740a7f 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -56,13 +56,13 @@ export function useState(initialState: (() => S) | S) { return dispatcher.useState(initialState); } -export function useReducer( +export function useReducer( reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialArg: I, + init?: I => S, ) { const dispatcher = resolveDispatcher(); - return dispatcher.useReducer(reducer, initialState, initialAction); + return dispatcher.useReducer(reducer, initialArg, init); } export function useRef(initialValue: T): {current: T} {