Make further bundle size optimisations (#375)

* Extract result/error generation from exchanges * Replace process.env.NODE_ENV in minified bundle This will give us accurate build numbers * Remove unnecessary rest spread from components This removes the __rest polyfill from our bundles. The rest spread isn't necessary since the props are compatible interfaces that extend the hooks' args. * Replace debug exchange with noop in production * Add missing result export in utils * (refactor) - avoid implicit returns where we have no value in returns like in forEach statements * (refactor) - avoid destructuring when the variable is only used once * (refactor) - convert the two step return of error.ts in a one step return
urql-graphql · Aug 12, 2019 · f30b28f · f30b28f
1 parent e781a28
commit f30b28f
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 109 deletions.
diff --git a/package.json b/package.json
@@ -122,6 +122,7 @@
     "rollup-plugin-buble": "^0.19.8",
     "rollup-plugin-commonjs": "^10.0.1",
     "rollup-plugin-node-resolve": "^5.2.0",
+    "rollup-plugin-replace": "^2.2.0",
     "rollup-plugin-terser": "^5.1.1",
     "rollup-plugin-typescript2": "^0.22.0",
     "terser": "^4.1.2",

diff --git a/rollup.config.js b/rollup.config.js
@@ -4,6 +4,7 @@ import nodeResolve from 'rollup-plugin-node-resolve';
 import typescript from 'rollup-plugin-typescript2';
 import buble from 'rollup-plugin-buble';
 import babel from 'rollup-plugin-babel';
+import replace from 'rollup-plugin-replace';
 import { terser } from 'rollup-plugin-terser';
 
 const pkgInfo = require('./package.json');
@@ -131,8 +132,11 @@ const makePlugins = (isProduction = false) => [
       }]
     ]
   }),
+  isProduction && replace({
+    'process.env.NODE_ENV': JSON.stringify('production')
+  }),
   isProduction ? terserMinified : terserPretty
-];
+].filter(Boolean);
 
 const config = {
   input: './src/index.ts',
@@ -167,16 +171,17 @@ export default [
   }, {
     ...config,
     plugins: makePlugins(true),
+    onwarn: () => {},
     output: [
       {
-        sourcemap: true,
+        sourcemap: false,
         legacy: true,
         freeze: false,
         file: './dist/urql.min.js',
         format: 'cjs'
       },
       {
-        sourcemap: true,
+        sourcemap: false,
         legacy: true,
         freeze: false,
         file: './dist/urql.es.min.js',

diff --git a/src/components/Query.ts b/src/components/Query.ts
@@ -10,10 +10,9 @@ export interface QueryState<T> extends UseQueryState<T> {
   executeQuery: (opts?: Partial<OperationContext>) => void;
 }
 
-export function Query<T = any, V = any>({
-  children,
-  ...args
-}: QueryProps<T, V>): ReactElement<any> {
-  const [state, executeQuery] = useQuery<T, V>(args);
-  return children({ ...state, executeQuery });
+export function Query<T = any, V = any>(
+  props: QueryProps<T, V>
+): ReactElement<any> {
+  const [state, executeQuery] = useQuery<T, V>(props);
+  return props.children({ ...state, executeQuery });
 }
diff --git a/src/components/Subscription.ts b/src/components/Subscription.ts
@@ -12,11 +12,9 @@ export interface SubscriptionProps<T, R, V> extends UseSubscriptionArgs<V> {
   children: (arg: UseSubscriptionState<R>) => ReactElement<any>;
 }
 
-export function Subscription<T = any, R = T, V = any>({
-  children,
-  handler,
-  ...args
-}: SubscriptionProps<T, R, V>): ReactElement<any> {
-  const [state] = useSubscription<T, R, V>(args, handler);
-  return children(state);
+export function Subscription<T = any, R = T, V = any>(
+  props: SubscriptionProps<T, R, V>
+): ReactElement<any> {
+  const [state] = useSubscription<T, R, V>(props, props.handler);
+  return props.children(state);
 }
diff --git a/src/exchanges/cache.ts b/src/exchanges/cache.ts
@@ -56,9 +56,8 @@ export const cacheExchange: Exchange = ({ forward, client }) => {
       sharedOps$,
       filter(op => !shouldSkip(op) && isOperationCached(op)),
       map(operation => {
-        const { key, context } = operation;
-        const cachedResult = resultCache.get(key);
-        if (context.requestPolicy === 'cache-and-network') {
+        const cachedResult = resultCache.get(operation.key);
+        if (operation.context.requestPolicy === 'cache-and-network') {
           reexecuteOperation(client, operation);
         }
 
@@ -135,7 +134,9 @@ export const afterMutation = (
   collectTypesFromResponse(response.data).forEach(typeName => {
     const operations =
       operationCache[typeName] || (operationCache[typeName] = new Set());
-    operations.forEach(key => pendingOperations.add(key));
+    operations.forEach(key => {
+      pendingOperations.add(key);
+    });
     operations.clear();
   });
 

diff --git a/src/exchanges/debug.ts b/src/exchanges/debug.ts
@@ -2,15 +2,19 @@ import { pipe, tap } from 'wonka';
 import { Exchange } from '../types';
 
 export const debugExchange: Exchange = ({ forward }) => {
-  return ops$ =>
-    pipe(
-      ops$,
-      // eslint-disable-next-line no-console
-      tap(op => console.log('[Exchange debug]: Incoming operation: ', op)),
-      forward,
-      tap(result =>
+  if (process.env.NODE_ENV === 'production') {
+    return ops$ => forward(ops$);
+  } else {
+    return ops$ =>
+      pipe(
+        ops$,
         // eslint-disable-next-line no-console
-        console.log('[Exchange debug]: Completed operation: ', result)
-      )
-    );
+        tap(op => console.log('[Exchange debug]: Incoming operation: ', op)),
+        forward,
+        tap(result =>
+          // eslint-disable-next-line no-console
+          console.log('[Exchange debug]: Completed operation: ', result)
+        )
+      );
+  }
 };
diff --git a/src/exchanges/fetch.ts b/src/exchanges/fetch.ts
@@ -2,7 +2,7 @@
 import { print } from 'graphql';
 import { filter, make, merge, mergeMap, pipe, share, takeUntil } from 'wonka';
 import { Exchange, Operation, OperationResult } from '../types';
-import { addMetadata, CombinedError } from '../utils';
+import { addMetadata, makeResult, makeErrorResult } from '../utils';
 
 /** A default exchange for fetching GraphQL requests. */
 export const fetchExchange: Exchange = ({ forward }) => {
@@ -41,7 +41,10 @@ export const fetchExchange: Exchange = ({ forward }) => {
 };
 
 const createFetchSource = (operation: Operation) => {
-  if (operation.operationName === 'subscription') {
+  if (
+    process.env.NODE_ENV !== 'production' &&
+    operation.operationName === 'subscription'
+  ) {
     throw new Error(
       'Received a subscription operation in the httpExchange. You are probably trying to create a subscription. Have you added a subscriptionExchange?'
     );
@@ -79,12 +82,16 @@ const createFetchSource = (operation: Operation) => {
 
     executeFetch(operation, fetchOptions).then(result => {
       if (result !== undefined) {
-        next({
-          ...result,
-          operation: addMetadata(result.operation, {
-            networkLatency: Date.now() - startTime,
-          }),
-        });
+        if (process.env.NODE_ENV !== 'production') {
+          next({
+            ...result,
+            operation: addMetadata(result.operation, {
+              networkLatency: Date.now() - startTime,
+            }),
+          });
+        } else {
+          next(result);
+        }
       }
 
       complete();
@@ -99,50 +106,26 @@ const createFetchSource = (operation: Operation) => {
 };
 
 const executeFetch = (operation: Operation, opts: RequestInit) => {
-  let response: Response | undefined;
   const { url, fetch: fetcher } = operation.context;
 
+  let response: Response | undefined;
+
   return (fetcher || fetch)(url, opts)
     .then(res => {
+      const { status } = res;
+      const statusRangeEnd = opts.redirect === 'manual' ? 400 : 300;
       response = res;
-      checkStatus(opts.redirect, res);
-      return res.json();
+
+      if (status < 200 || status >= statusRangeEnd) {
+        throw new Error(res.statusText);
+      } else {
+        return res.json();
+      }
     })
-    .then(result => ({
-      operation,
-      data: result.data,
-      error: Array.isArray(result.errors)
-        ? new CombinedError({
-            graphQLErrors: result.errors,
-            response,
-          })
-        : undefined,
-      extensions:
-        typeof result.extensions === 'object' && result.extensions !== null
-          ? result.extensions
-          : undefined,
-    }))
+    .then(result => makeResult(operation, result, response))
     .catch(err => {
-      if (err.name === 'AbortError') {
-        return undefined;
+      if (err.name !== 'AbortError') {
+        return makeErrorResult(operation, err, response);
       }
-
-      return {
-        operation,
-        data: undefined,
-        error: new CombinedError({
-          networkError: err,
-          response,
-        }),
-        extensions: undefined,
-      };
     });
 };
-
-const checkStatus = (redirectMode: string = 'follow', response: Response) => {
-  const statusRangeEnd = redirectMode === 'manual' ? 400 : 300;
-
-  if (response.status < 200 || response.status >= statusRangeEnd) {
-    throw new Error(response.statusText);
-  }
-};
diff --git a/src/exchanges/subscription.ts b/src/exchanges/subscription.ts
@@ -11,7 +11,7 @@ import {
   takeUntil,
 } from 'wonka';
 
-import { CombinedError } from '../utils/error';
+import { makeResult, makeErrorResult } from '../utils';
 
 import {
   Exchange,
@@ -72,39 +72,12 @@ export const subscriptionExchange = ({
     });
 
     return make<OperationResult>(([next, complete]) => {
-      // TODO: The conversion of the result here is very similar to fetch;
-      // We can maybe extract the logic into generic GraphQL utilities
       const sub = observableish.subscribe({
-        next: result =>
-          next({
-            operation,
-            data: result.data || undefined,
-            error: Array.isArray(result.errors)
-              ? new CombinedError({
-                  graphQLErrors: result.errors,
-                  response: undefined,
-                })
-              : undefined,
-            extensions:
-              typeof result.extensions === 'object' &&
-              result.extensions !== null
-                ? result.extensions
-                : undefined,
-          }),
-        error: err =>
-          next({
-            operation,
-            data: undefined,
-            extensions: undefined,
-            error: new CombinedError({
-              networkError: err,
-              response: undefined,
-            }),
-          }),
+        next: result => next(makeResult(operation, result)),
+        error: err => next(makeErrorResult(operation, err)),
         complete,
       });
 
-      // NOTE: Destructuring sub is avoided here to preserve its potential binding
       return () => sub.unsubscribe();
     });
   };

diff --git a/src/utils/error.ts b/src/utils/error.ts
@@ -6,8 +6,7 @@ const generateErrorMessage = (
 ) => {
   let error = '';
   if (networkErr !== undefined) {
-    error = `[Network] ${networkErr.message}`;
-    return error;
+    return (error = `[Network] ${networkErr.message}`);
   }
 
   if (graphQlErrs !== undefined) {

diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -1,5 +1,6 @@
 export * from './error';
 export * from './request';
+export * from './result';
 export * from './typenames';
 export * from './toSuspenseSource';
 

diff --git a/src/utils/result.ts b/src/utils/result.ts
@@ -0,0 +1,35 @@
+import { Operation, OperationResult } from '../types';
+import { CombinedError } from './error';
+
+export const makeResult = (
+  operation: Operation,
+  result: any,
+  response?: any
+): OperationResult => ({
+  operation,
+  data: result.data,
+  error: Array.isArray(result.errors)
+    ? new CombinedError({
+        graphQLErrors: result.errors,
+        response,
+      })
+    : undefined,
+  extensions:
+    typeof result.extensions === 'object' && result.extensions !== null
+      ? result.extensions
+      : undefined,
+});
+
+export const makeErrorResult = (
+  operation: Operation,
+  error: Error,
+  response?: any
+): OperationResult => ({
+  operation,
+  data: undefined,
+  error: new CombinedError({
+    networkError: error,
+    response,
+  }),
+  extensions: undefined,
+});
diff --git a/src/utils/typenames.ts b/src/utils/typenames.ts
@@ -13,7 +13,9 @@ interface EntityLike {
 
 const collectTypes = (obj: EntityLike | EntityLike[], types: string[] = []) => {
   if (Array.isArray(obj)) {
-    obj.forEach(inner => collectTypes(inner, types));
+    obj.forEach(inner => {
+      collectTypes(inner, types);
+    });
   } else if (typeof obj === 'object' && obj !== null) {
     for (const key in obj) {
       if (Object.prototype.hasOwnProperty.call(obj, key)) {

diff --git a/yarn.lock b/yarn.lock
@@ -5279,6 +5279,14 @@ rollup-plugin-node-resolve@^5.2.0:
     resolve "^1.11.1"
     rollup-pluginutils "^2.8.1"
 
+rollup-plugin-replace@^2.2.0:
+  version "2.2.0"
+  resolved "https://registry.yarnpkg.com/rollup-plugin-replace/-/rollup-plugin-replace-2.2.0.tgz#f41ae5372e11e7a217cde349c8b5d5fd115e70e3"
+  integrity sha512-/5bxtUPkDHyBJAKketb4NfaeZjL5yLZdeUihSfbF2PQMz+rSTEb8ARKoOl3UBT4m7/X+QOXJo3sLTcq+yMMYTA==
+  dependencies:
+    magic-string "^0.25.2"
+    rollup-pluginutils "^2.6.0"
+
 rollup-plugin-terser@^5.1.1:
   version "5.1.1"
   resolved "https://registry.yarnpkg.com/rollup-plugin-terser/-/rollup-plugin-terser-5.1.1.tgz#e9d2545ec8d467f96ba99b9216d2285aad8d5b66"
@@ -5300,7 +5308,7 @@ rollup-plugin-typescript2@^0.22.0:
     rollup-pluginutils "2.8.1"
     tslib "1.10.0"
 
-rollup-pluginutils@2.8.1, rollup-pluginutils@^2.8.1:
+rollup-pluginutils@2.8.1, rollup-pluginutils@^2.6.0, rollup-pluginutils@^2.8.1:
   version "2.8.1"
   resolved "https://registry.yarnpkg.com/rollup-pluginutils/-/rollup-pluginutils-2.8.1.tgz#8fa6dd0697344938ef26c2c09d2488ce9e33ce97"
   integrity sha512-J5oAoysWar6GuZo0s+3bZ6sVZAC0pfqKz68De7ZgDi5z63jOVZn1uJL/+z1jeKHNbGII8kAyHF5q8LnxSX5lQg==