Repository: FormidableLabs/urql
Pull request #310 (2019-06-18 09:44:00)

Fix broken SSR on master
It looks like the SSR reproduction on the current master is broken (looks to be related to #296).

```
UnhandledPromiseRejectionWarning: Error: [react-ssr-prepass] Hooks can only be called inside the body of a function component. (https://fb.me/react-invalid-hook-call)
```

To get around the late call of useEffect (causing problems in both SSR and browser environments) we now use useLayoutEffect.

This also addresses the issue I've been seeing locally where super fast network responses cause the state not to be updated.

JoviDeCroock: If you want I can test it against the reproduction case provided in: https://codesandbox.io/s/nostalgic-sound-psbzf?fontsize=14 
andyrichardson: @JoviDeCroock that's awesome! I'll give it a shot

Patch:
diff --git a/.sail.yml b/.sail.yml
index faf0f5e9..f315f9e9 100644
--- a/.sail.yml
+++ b/.sail.yml
@@ -2,6 +2,7 @@ workflow:
   - install
   - sail:parallel:
     - build
+    - check
     - lint
     - test
     - e2e
@@ -36,6 +37,12 @@ tasks:
       - yarn
     args:
       - build
+  check:
+    image: node:10
+    command:
+      - yarn
+    args:
+      - check
   e2e:
     image: cypress/browsers:node10.2.1-chrome74
     command:
diff --git a/package.json b/package.json
index d70e05f7..e31ef4e6 100644
--- a/package.json
+++ b/package.json
@@ -21,17 +21,14 @@
     "exchanges"
   ],
   "scripts": {
-    "prebuild": "rimraf dist",
-    "build": "run-p build:types build:bundle",
-    "build:clean": "rimraf dist",
-    "build:types": "tsc -d --emitDeclarationOnly --outDir dist/types",
-    "build:bundle": "rollup -c rollup.config.js",
-    "build:prune": "rimraf dist/types/**/*.test.d.ts dist/types/test-utils",
+    "clean": "rimraf ./dist ./node_modules/.cache",
+    "build": "rollup -c rollup.config.js",
+    "watch": "rollup -w -c rollup.config.js",
+    "check": "tsc --noEmit",
     "test": "jest",
     "coverage": "jest --coverage",
     "lint": "eslint . --ext .ts,.tsx",
-    "check-formatting": "prettier --write src/**/*.{ts,tsx}",
-    "prepublishOnly": "run-s build build:prune",
+    "prepublishOnly": "run-s clean test build",
     "codecov": "codecov"
   },
   "author": "Formidable",
diff --git a/rollup.config.js b/rollup.config.js
index aa2905c2..26b36e03 100644
--- a/rollup.config.js
+++ b/rollup.config.js
@@ -69,7 +69,7 @@ const terserMinified = terser({
   }
 });
 
-const plugins = [
+const makePlugins = (isProduction = false) => [
   nodeResolve({
     mainFields: ['module', 'jsnext', 'main'],
     browser: true
@@ -84,14 +84,21 @@ const plugins = [
   typescript({
     typescript: require('typescript'),
     cacheRoot: './node_modules/.cache/.rts2_cache',
+    useTsconfigDeclarationDir: true,
     tsconfigDefaults: {
       compilerOptions: {
-        sourceMap: true,
-        declaration: true
+        sourceMap: true
       },
     },
     tsconfigOverride: {
-      compilerOptions: {
+     exclude: [
+       'src/**/*.test.ts',
+       'src/**/*.test.tsx',
+       'src/**/test-utils/*'
+     ],
+     compilerOptions: {
+        declaration: !isProduction,
+        declarationDir: './dist/types/',
         target: 'es6',
       },
     },
@@ -123,7 +130,8 @@ const plugins = [
         externalHelpers: true
       }]
     ]
-  })
+  }),
+  isProduction ? terserMinified : terserPretty
 ];
 
 const config = {
@@ -137,7 +145,7 @@ const config = {
 export default [
   {
     ...config,
-    plugins: [...plugins, terserPretty],
+    plugins: makePlugins(false),
     output: [
       {
         sourcemap: true,
@@ -158,7 +166,7 @@ export default [
     ]
   }, {
     ...config,
-    plugins: [...plugins, terserMinified],
+    plugins: makePlugins(true),
     output: [
       {
         sourcemap: true,
diff --git a/src/utils/index.ts b/src/utils/index.ts
index 4d56129e..99f7b9aa 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 00000000..9eef157d
--- /dev/null
+++ b/src/utils/ssr.ts
@@ -0,0 +1,2 @@
+export const isSSR =
+  typeof window === 'undefined' || !('HTMLElement' in window);


Test patch:
diff --git a/src/hooks/useImmediateState.test.ts b/src/hooks/useImmediateState.test.ts
deleted file mode 100644
index e1e6db40..00000000
--- 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 00000000..95ddf999
--- /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<object>(initialState);
+
+  if (update) {
+    set(updateState);
+  }
+
+  state = a;
+  setState = set;
+
+  return null;
+};
+
+beforeEach(jest.clearAllMocks);
+
+describe('on initial mount', () => {
+  it('sets initial state', () => {
+    renderer.create(<Fixture />);
+    expect(state).toEqual(initialState);
+  });
+
+  it('only mutates on setState call', () => {
+    renderer.create(<Fixture update={true} />);
+    expect(setStateMock).toBeCalledTimes(0);
+    expect(state).toEqual(updateState);
+  });
+});
+
+describe('on later mounts', () => {
+  it('sets state via setState', () => {
+    renderer.create(<Fixture />);
+    setState(updateState);
+    expect(setStateMock).toBeCalledTimes(1);
+  });
+});
diff --git a/src/hooks/useImmediateState.ts b/src/hooks/useImmediateState.ts
index 305a0a34..c1bc9bec 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> = S | ((prevState: S) => S);
 type SetState<S> = (action: SetStateAction<S>) => 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 = <S extends {}>(init: S): [S, SetState<S>] => {
   const isMounted = useRef(false);
   const initialState = useRef<S>({ ...init });
   const [state, setState] = useState<S>(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<S> = useCallback((action: SetStateAction<S>) => {
-    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];
 };