From 40e371bd7d3fdbca2b7c38ba63fe331f7d4bc6d5 Mon Sep 17 00:00:00 2001 From: Andy Richardson Date: Tue, 18 Jun 2019 13:39:54 +0100 Subject: [PATCH] Fix broken SSR on master (#310) * fix broken ssr * update tests * update tests * remove ssr abstraction * don't call useLayoutEffect in SSR * update SSR check * ssr.... --- src/hooks/useImmediateState.test.ts | 46 --------------------------- src/hooks/useImmediateState.test.tsx | 47 ++++++++++++++++++++++++++++ src/hooks/useImmediateState.ts | 44 +++++++++++++------------- src/utils/index.ts | 1 + src/utils/ssr.ts | 2 ++ 5 files changed, 72 insertions(+), 68 deletions(-) delete mode 100644 src/hooks/useImmediateState.test.ts create mode 100644 src/hooks/useImmediateState.test.tsx create mode 100644 src/utils/ssr.ts diff --git a/src/hooks/useImmediateState.test.ts b/src/hooks/useImmediateState.test.ts deleted file mode 100644 index e1e6db404d..0000000000 --- a/src/hooks/useImmediateState.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import React from 'react'; -import { renderHook } from 'react-hooks-testing-library'; -import { useImmediateState } from './useImmediateState'; - -it('updates state immediately during mount', () => { - let initialState; - let update = 0; - - const setState = jest.fn(); - - const spy = jest.spyOn(React, 'useState').mockImplementation(state => { - expect(state).toEqual({ x: 'x', test: false }); - initialState = state; - return [state, setState]; - }); - - const { result } = renderHook(() => { - const [state, setState] = useImmediateState({ x: 'x', test: false }); - if (update === 0) setState(s => ({ ...s, test: true })); - update++; - return state; - }); - - expect(setState).not.toHaveBeenCalled(); - expect(result.current).toEqual({ x: 'x', test: true }); - expect(result.current).toBe(initialState); - expect(update).toBe(1); - - spy.mockRestore(); -}); - -it('behaves like useState otherwise', () => { - const setState = jest.fn(); - const spy = jest - .spyOn(React, 'useState') - .mockImplementation(state => [state, setState]); - - renderHook(() => { - const [state, setState] = useImmediateState({ x: 'x' }); - React.useEffect(() => setState({ x: 'y' }), [setState]); - return state; - }); - - expect(setState).toHaveBeenCalledTimes(1); - spy.mockRestore(); -}); diff --git a/src/hooks/useImmediateState.test.tsx b/src/hooks/useImmediateState.test.tsx new file mode 100644 index 0000000000..95ddf999ec --- /dev/null +++ b/src/hooks/useImmediateState.test.tsx @@ -0,0 +1,47 @@ +import React from 'react'; +import renderer from 'react-test-renderer'; +import { useImmediateState } from './useImmediateState'; + +const setStateMock = jest.fn(); +jest.spyOn(React, 'useState').mockImplementation(arg => [arg, setStateMock]); + +const initialState = { someObject: 1234 }; +const updateState = { someObject: 5678 }; +let state; +let setState; + +const Fixture = ({ update }: { update?: boolean }) => { + const [a, set] = useImmediateState(initialState); + + if (update) { + set(updateState); + } + + state = a; + setState = set; + + return null; +}; + +beforeEach(jest.clearAllMocks); + +describe('on initial mount', () => { + it('sets initial state', () => { + renderer.create(); + expect(state).toEqual(initialState); + }); + + it('only mutates on setState call', () => { + renderer.create(); + expect(setStateMock).toBeCalledTimes(0); + expect(state).toEqual(updateState); + }); +}); + +describe('on later mounts', () => { + it('sets state via setState', () => { + renderer.create(); + setState(updateState); + expect(setStateMock).toBeCalledTimes(1); + }); +}); diff --git a/src/hooks/useImmediateState.ts b/src/hooks/useImmediateState.ts index 305a0a34d1..c1bc9bec8b 100644 --- a/src/hooks/useImmediateState.ts +++ b/src/hooks/useImmediateState.ts @@ -1,39 +1,39 @@ -import { useRef, useEffect, useState, useCallback } from 'react'; +import { useRef, useState, useCallback, useLayoutEffect } from 'react'; +import { isSSR } from '../utils'; type SetStateAction = S | ((prevState: S) => S); type SetState = (action: SetStateAction) => void; -/** This is a drop-in replacement for useState, limited to object-based state. During initial mount it will mutably update the state, instead of scheduling a React update using setState */ +/** + * This is a drop-in replacement for useState, limited to object-based state. + * During initial mount it will mutably update the state, instead of scheduling + * a React update using setState + */ export const useImmediateState = (init: S): [S, SetState] => { const isMounted = useRef(false); const initialState = useRef({ ...init }); const [state, setState] = useState(initialState.current); // This wraps setState and updates the state mutably on initial mount - // It also prevents setting the state when the component is unmounted const updateState: SetState = useCallback((action: SetStateAction) => { - if (isMounted.current) { - setState(action); - } else if (typeof action === 'function') { - const update = (action as any)(initialState.current); - Object.assign(initialState.current, update); - } else { - // There are scenario's where we are in-between mounts. - // The reason we need both is because we COULD still be mounting - // and we could be in the process of being mounted. - // Scenario 1 needs the initialState.current mutation. - // Scenario 2 needs the setState. - // See https://github.com/FormidableLabs/urql/issues/287 - setState(() => Object.assign(initialState.current, action as any)); + if (!isMounted.current) { + const newValue = + typeof action === 'function' + ? (action as (arg: S) => S)(initialState.current) + : action; + return Object.assign(initialState.current, newValue); } - }, []); - useEffect(() => { - isMounted.current = true; - return () => { - isMounted.current = false; - }; + setState(action); }, []); + !isSSR && // eslint-disable-next-line react-hooks/rules-of-hooks + useLayoutEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); + return [state, updateState]; }; diff --git a/src/utils/index.ts b/src/utils/index.ts index 4d56129e46..99f7b9aaa9 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,6 +1,7 @@ export { CombinedError } from './error'; export { getKeyForRequest } from './keyForQuery'; export { createRequest } from './request'; +export * from './ssr'; export { formatDocument, collectTypesFromResponse } from './typenames'; export { toSuspenseSource } from './toSuspenseSource'; diff --git a/src/utils/ssr.ts b/src/utils/ssr.ts new file mode 100644 index 0000000000..9eef157da6 --- /dev/null +++ b/src/utils/ssr.ts @@ -0,0 +1,2 @@ +export const isSSR = + typeof window === 'undefined' || !('HTMLElement' in window);