chore(core): Refactor core code and remove modifiable Client options

.changeset/six-rules-study.md

@@ -0,0 +1,5 @@
+---
+'@urql/core': major
+---
+
+Remove support for options on the `Client` and `Client.createOperationContext`. We've noticed that there's no real need for `createOperationContext` or the options on the `Client` and that it actually encourages modifying properties on the `Client` that are really meant to be modified dynamically via exchanges.

packages/core/src/__snapshots__/client.test.ts.snap

@@ -2,25 +2,18 @@
 
 exports[`createClient / Client passes snapshot 1`] = `
 Client {
-  "createOperationContext": [Function],
   "createRequestOperation": [Function],
   "executeMutation": [Function],
   "executeQuery": [Function],
   "executeRequestOperation": [Function],
   "executeSubscription": [Function],
-  "fetch": undefined,
-  "fetchOptions": undefined,
-  "maskTypename": false,
   "mutation": [Function],
   "operations$": [Function],
-  "preferGetMethod": false,
   "query": [Function],
   "readQuery": [Function],
   "reexecuteOperation": [Function],
-  "requestPolicy": "cache-first",
   "subscribeToDebugTarget": [Function],
   "subscription": [Function],
   "suspense": false,
-  "url": "https://hostname.com",
 }
 `;

packages/core/src/client.ts

@@ -74,25 +74,13 @@ export interface Client {
   new (options: ClientOptions): Client;
 
   operations$: Source<Operation>;
+  suspense: boolean;
 
   /** Start an operation from an exchange */
   reexecuteOperation: (operation: Operation) => void;
   /** Event target for monitoring, e.g. for @urql/devtools */
   subscribeToDebugTarget?: (onEvent: (e: DebugEvent) => void) => Subscription;
 
-  // These are variables derived from ClientOptions
-  url: string;
-  fetch?: typeof fetch;
-  fetchOptions?: RequestInit | (() => RequestInit);
-  suspense: boolean;
-  requestPolicy: RequestPolicy;
-  preferGetMethod: boolean;
-  maskTypename: boolean;
-
-  createOperationContext(
-    opts?: Partial<OperationContext> | undefined
-  ): OperationContext;
-
   createRequestOperation<
     Data = any,
     Variables extends AnyVariables = AnyVariables
@@ -165,6 +153,14 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
   const active: Map<number, Source<OperationResult>> = new Map();
   const queue: Operation[] = [];
 
+  const baseOpts = {
+    url: opts.url,
+    fetchOptions: opts.fetchOptions,
+    fetch: opts.fetch,
+    preferGetMethod: !!opts.preferGetMethod,
+    requestPolicy: opts.requestPolicy || 'cache-first',
+  };
+
   // This subject forms the input of operations; executeOperation may be
   // called to dispatch a new operation on the subject
   const { source: operations$, next: nextOperation } = makeSubject<Operation>();
@@ -197,7 +193,7 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
     );
 
     // Mask typename properties if the option for it is turned on
-    if (client.maskTypename) {
+    if (opts.maskTypename) {
       result$ = pipe(
         result$,
         map(res => ({ ...res, data: maskTypename(res.data) }))
@@ -265,14 +261,7 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
   const instance: Client =
     this instanceof Client ? this : Object.create(Client.prototype);
   const client: Client = Object.assign(instance, {
-    url: opts.url,
-    fetchOptions: opts.fetchOptions,
-    fetch: opts.fetch,
     suspense: !!opts.suspense,
-    requestPolicy: opts.requestPolicy || 'cache-first',
-    preferGetMethod: !!opts.preferGetMethod,
-    maskTypename: !!opts.maskTypename,
-
     operations$,
 
     reexecuteOperation(operation: Operation) {
@@ -284,22 +273,8 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
       }
     },
 
-    createOperationContext(opts) {
-      if (!opts) opts = {};
-
-      return {
-        _instance: undefined,
-        url: client.url,
-        fetchOptions: client.fetchOptions,
-        fetch: client.fetch,
-        preferGetMethod: client.preferGetMethod,
-        ...opts,
-        suspense: opts.suspense || (opts.suspense !== false && client.suspense),
-        requestPolicy: opts.requestPolicy || client.requestPolicy,
-      };
-    },
-
     createRequestOperation(kind, request, opts) {
+      if (!opts) opts = {};
       const requestOperationType = getOperationType(request.query);
       if (
         process.env.NODE_ENV !== 'production' &&
@@ -310,9 +285,12 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
           `Expected operation of type "${kind}" but found "${requestOperationType}"`
         );
       }
-      const context = client.createOperationContext(opts);
-      if (kind === 'mutation') (context as any)._instance = [];
-      return makeOperation(kind, request, context);
+      return makeOperation(kind, request, {
+        _instance: kind === 'mutation' ? [] : undefined,
+        ...baseOpts,
+        ...opts,
+        suspense: opts.suspense || (opts.suspense !== false && client.suspense),
+      });
     },
 
     executeRequestOperation(operation) {

packages/core/src/exchanges/fetch.ts

@@ -20,11 +20,6 @@ export const fetchExchange: Exchange = ({ forward, dispatchDebug }) => {
       }),
       mergeMap(operation => {
         const { key } = operation;
-        const teardown$ = pipe(
-          sharedOps$,
-          filter(op => op.kind === 'teardown' && op.key === key)
-        );
-
         const body = makeFetchBody(operation);
         const url = makeFetchURL(operation, body);
         const fetchOptions = makeFetchOptions(operation, body);
@@ -41,7 +36,12 @@ export const fetchExchange: Exchange = ({ forward, dispatchDebug }) => {
 
         const source = pipe(
           makeFetchSource(operation, url, fetchOptions),
-          takeUntil(teardown$)
+          takeUntil(
+            pipe(
+              sharedOps$,
+              filter(op => op.kind === 'teardown' && op.key === key)
+            )
+          )
         );
 
         if (process.env.NODE_ENV !== 'production') {

packages/core/src/exchanges/ssr.ts

@@ -91,9 +91,9 @@ const deserializeResult = (
 const revalidated = new Set<number>();
 
 /** The ssrExchange can be created to capture data during SSR and also to rehydrate it on the client */
-export const ssrExchange = (params?: SSRExchangeParams): SSRExchange => {
-  const staleWhileRevalidate = !!(params && params.staleWhileRevalidate);
-  const includeExtensions = !!(params && params.includeExtensions);
+export const ssrExchange = (params: SSRExchangeParams = {}): SSRExchange => {
+  const staleWhileRevalidate = !!params.staleWhileRevalidate;
+  const includeExtensions = !!params.includeExtensions;
   const data: Record<string, SerializedResult | null> = {};
 
   // On the client-side, we delete results from the cache as they're resolved

packages/core/src/gql.ts

@@ -1,28 +1,21 @@
 /* eslint-disable prefer-rest-params */
 import { TypedDocumentNode } from '@graphql-typed-document-node/core';
-
-import {
-  DocumentNode,
-  DefinitionNode,
-  FragmentDefinitionNode,
-  Kind,
-} from 'graphql';
-
+import { DocumentNode, DefinitionNode, Kind } from 'graphql';
 import { keyDocument, stringifyDocument } from './utils';
 
 const applyDefinitions = (
   fragmentNames: Map<string, string>,
   target: DefinitionNode[],
   source: Array<DefinitionNode> | ReadonlyArray<DefinitionNode>
 ) => {
-  for (let i = 0; i < source.length; i++) {
-    if (source[i].kind === Kind.FRAGMENT_DEFINITION) {
-      const name = (source[i] as FragmentDefinitionNode).name.value;
-      const value = stringifyDocument(source[i]);
+  for (const definition of source) {
+    if (definition.kind === Kind.FRAGMENT_DEFINITION) {
+      const name = definition.name.value;
+      const value = stringifyDocument(definition);
       // Fragments will be deduplicated according to this Map
       if (!fragmentNames.has(name)) {
         fragmentNames.set(name, value);
-        target.push(source[i]);
+        target.push(definition);
       } else if (
         process.env.NODE_ENV !== 'production' &&
         fragmentNames.get(name) !== value
@@ -36,7 +29,7 @@ const applyDefinitions = (
         );
       }
     } else {
-      target.push(source[i]);
+      target.push(definition);
     }
   }
 };

packages/core/src/internal/fetchOptions.ts

@@ -9,10 +9,6 @@ export interface FetchBody {
   extensions: undefined | Record<string, any>;
 }
 
-const shouldUseGet = (operation: Operation): boolean => {
-  return operation.kind === 'query' && !!operation.context.preferGetMethod;
-};
-
 export function makeFetchBody<
   Data = any,
   Variables extends AnyVariables = AnyVariables
@@ -29,7 +25,8 @@ export const makeFetchURL = (
   operation: Operation,
   body?: FetchBody
 ): string => {
-  const useGETMethod = shouldUseGet(operation);
+  const useGETMethod =
+    operation.kind === 'query' && !!operation.context.preferGetMethod;
   if (!useGETMethod || !body) return operation.context.url;
 
   const url = new URL(operation.context.url);
@@ -55,7 +52,8 @@ export const makeFetchOptions = (
   operation: Operation,
   body?: FetchBody
 ): RequestInit => {
-  const useGETMethod = shouldUseGet(operation);
+  const useGETMethod =
+    operation.kind === 'query' && !!operation.context.preferGetMethod;
   const headers: HeadersInit = {
     accept: 'application/graphql+json, application/json',
   };

packages/core/src/internal/fetchSource.ts

@@ -2,8 +2,6 @@ import { Source, make } from 'wonka';
 import { Operation, OperationResult } from '../types';
 import { makeResult, makeErrorResult, mergeResultPatch } from '../utils';
 
-const asyncIterator =
-  typeof Symbol !== 'undefined' ? Symbol.asyncIterator : null;
 const decoder = typeof TextDecoder !== 'undefined' ? new TextDecoder() : null;
 const jsonHeaderRe = /content-type:[^\r\n]*application\/json/i;
 const boundaryHeaderRe = /boundary="?([^=";]+)"?/i;
@@ -61,13 +59,13 @@ export const makeFetchSource = (
       let cancel = () => {
         /*noop*/
       };
-      if (asyncIterator && response[asyncIterator]) {
-        const iterator = response[asyncIterator]();
+      if (response[Symbol.asyncIterator]) {
+        const iterator = response[Symbol.asyncIterator]();
         read = iterator.next.bind(iterator);
       } else if ('body' in response && response.body) {
         const reader = response.body.getReader();
-        cancel = reader.cancel.bind(reader);
-        read = reader.read.bind(reader);
+        cancel = () => reader.cancel();
+        read = () => reader.read();
       } else {
         throw new TypeError('Streaming requests unsupported');
       }

packages/core/src/utils/error.ts

@@ -5,17 +5,14 @@ const generateErrorMessage = (
   graphQlErrs?: GraphQLError[]
 ) => {
   let error = '';
-  if (networkErr) {
-    return (error = `[Network] ${networkErr.message}`);
-  }
-
+  if (networkErr) return `[Network] ${networkErr.message}`;
   if (graphQlErrs) {
     for (const err of graphQlErrs) {
-      error += `[GraphQL] ${err.message}\n`;
+      if (error) error += '\n';
+      error += `[GraphQL] ${err.message}`;
     }
   }
-
-  return error.trim();
+  return error;
 };
 
 const rehydrateGraphQlError = (error: any): GraphQLError => {
@@ -44,27 +41,26 @@ export class CombinedError extends Error {
   public networkError?: Error;
   public response?: any;
 
-  constructor({
-    networkError,
-    graphQLErrors,
-    response,
-  }: {
+  constructor(input: {
     networkError?: Error;
     graphQLErrors?: Array<string | Partial<GraphQLError> | Error>;
     response?: any;
   }) {
-    const normalizedGraphQLErrors = (graphQLErrors || []).map(
+    const normalizedGraphQLErrors = (input.graphQLErrors || []).map(
       rehydrateGraphQlError
     );
-    const message = generateErrorMessage(networkError, normalizedGraphQLErrors);
+    const message = generateErrorMessage(
+      input.networkError,
+      normalizedGraphQLErrors
+    );
 
     super(message);
 
     this.name = 'CombinedError';
     this.message = message;
     this.graphQLErrors = normalizedGraphQLErrors;
-    this.networkError = networkError;
-    this.response = response;
+    this.networkError = input.networkError;
+    this.response = input.response;
   }
 
   toString() {

packages/core/src/utils/hash.ts

@@ -2,12 +2,9 @@
 // version of djb2 where we pretend that we're still looping over
 // the same string
 export const phash = (h: number, x: string): number => {
-  h = h | 0;
-  for (let i = 0, l = x.length | 0; i < l; i++) {
+  for (let i = 0, l = x.length | 0; i < l; i++)
     h = (h << 5) + h + x.charCodeAt(i);
-  }
-
-  return h;
+  return h | 0;
 };
 
 // This is a djb2 hashing function

packages/core/src/utils/maskTypename.ts

@@ -1,21 +1,22 @@
 export const maskTypename = (data: any): any => {
-  if (!data || typeof data !== 'object') return data;
-
-  const acc = Array.isArray(data) ? [] : {};
-  for (const key in data) {
-    const value = data[key];
-    if (key === '__typename') {
-      Object.defineProperty(acc, '__typename', {
-        enumerable: false,
-        value,
-      });
-    } else if (Array.isArray(value)) {
-      acc[key] = value.map(maskTypename);
-    } else if (value && typeof value === 'object' && '__typename' in value) {
-      acc[key] = maskTypename(value);
-    } else {
-      acc[key] = value;
+  if (!data || typeof data !== 'object') {
+    return data;
+  } else if (Array.isArray(data)) {
+    return data.map(maskTypename);
+  } else if (data && typeof data === 'object' && '__typename' in data) {
+    const acc = {};
+    for (const key in data) {
+      if (key === '__typename') {
+        Object.defineProperty(acc, '__typename', {
+          enumerable: false,
+          value: data.__typename,
+        });
+      } else {
+        acc[key] = maskTypename(data[key]);
+      }
     }
+    return acc;
+  } else {
+    return data;
   }
-  return acc;
 };