Repository: FormidableLabs/urql (ts)

Issue:
RFC: Allow root types override without Schema Awareness
<!--
  🚨 RFCs are for proposed changes (not bugs or questions)
  Specifically they are whenever you'd like to see new features
  being added to urql, or enable new use-cases.
  They're also relevant whenever @urql/core is changed, since
  changes to e.g. the `Client` or `Exchange` types are kept rare
  and lean.
-->

## Summary

Allow root types override without Schema Awareness

## Proposed Solution

In our GraphQL schema we have a type called Subscription. It is not a gql subscription, but rather a representation of a certain business entity (we work in the e-commerce domain). It was an aware choice of naming, as there is not planned support for gql subscription in the API.

Unfortunately, every time we fetch something of that type we receive `data=null` and this [error](https://formidable.com/open-source/urql/docs/graphcache/errors/#25-invalid-root-traversal).

We were able to fix that by overriding root subscription type in schema and enabling schema awareness. However, it came with a cost of either running introspection query on each app load (~1-2 secs of blocking load time) or baking it into the `json` file as recommended. Unfortunately, in our case API is developed by independent team, so keeping `schema.json` up to date is a bit of a challenge.

Is it possible to provide an easier way for us to override root subscription type? Guess that can be applied to other root types as well. We do not benefit much from Schema Awareness feature (which is nevertheless great), but instead we have to live with a hustle of providing a valid schema only for the subscription type override.


Pull request:
(graphcache) - Allow partial schema to be passed for altered root names
Resolve #1378

## Summary

I had some of these changes done in a forgotten branch but never proposed / opened this since we didn't have a corresponding RFC just yet, and I didn't have time to write it up.

Basically, we should allow the root names to be altered without enabling full schema awareness for exceptional cases where schema awareness is explicitly not wanted. This is rather rare with the advent of GraphQL Code Generator's built-in plugin to resolve this at build time / deploy time but it may be helpful still.

It's now possible to pass only the root names like so:

```js
const introspection = {
  __schema: {
    queryType: {
      name: 'query_root',
    },
    mutationType: {
      name: 'mutation_root',
    },
    subscriptionType: {
      name: 'subscription_root',
    },
  },
};

cacheExchange({ schema: introspection });
```

## Set of changes

- Make `SchemaIntrospector.types` optional and don't add it for partial schema introspection results
- Prevent `this.schema` from being set on `Store` when `SchemaIntrospector.types` is missing
- Update types in `schemaPredicates.ts` and fix up a test for this use-case
- Allow a partial introspection result to be passed to `schema`

Patch:
diff --git a/.changeset/blue-bees-join.md b/.changeset/blue-bees-join.md
new file mode 100644
index 00000000..77e51d30
--- /dev/null
+++ b/.changeset/blue-bees-join.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': minor
+---
+
+Allow `schema` option to be passed with a partial introspection result that only contains `queryType`, `mutationType`, and `subscriptionType` with their respective names. This allows you to pass `{ __schema: { queryType: { name: 'Query' } } }` and the likes to Graphcache's `cacheExchange` to alter the default root names without enabling full schema awareness.
diff --git a/.changeset/small-apricots-exercise.md b/.changeset/small-apricots-exercise.md
new file mode 100644
index 00000000..719cb374
--- /dev/null
+++ b/.changeset/small-apricots-exercise.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-request-policy': patch
+---
+
+Fix non-query operations being upgraded by `requestPolicyExchange` and time being stored by last issuance rather than last result.
diff --git a/exchanges/graphcache/src/ast/schema.ts b/exchanges/graphcache/src/ast/schema.ts
index 82f526d7..a0b7999b 100644
--- a/exchanges/graphcache/src/ast/schema.ts
+++ b/exchanges/graphcache/src/ast/schema.ts
@@ -1,5 +1,6 @@
 import {
   IntrospectionQuery,
+  IntrospectionSchema,
   IntrospectionInputValue,
   IntrospectionTypeRef,
   IntrospectionType,
@@ -28,13 +29,24 @@ export interface SchemaIntrospector {
   query: string | null;
   mutation: string | null;
   subscription: string | null;
-  types: Record<string, SchemaObject | SchemaUnion>;
+  types?: Record<string, SchemaObject | SchemaUnion>;
   isSubType(abstract: string, possible: string): boolean;
 }
 
+export interface PartialIntrospectionSchema {
+  queryType: { name: string; kind?: any };
+  mutationType?: { name: string; kind?: any };
+  subscriptionType?: { name: string; kind?: any };
+  types?: IntrospectionSchema['types'];
+}
+
+export type IntrospectionData =
+  | IntrospectionQuery
+  | { __schema: PartialIntrospectionSchema };
+
 export const buildClientSchema = ({
   __schema,
-}: IntrospectionQuery): SchemaIntrospector => {
+}: IntrospectionData): SchemaIntrospector => {
   const typemap: Record<string, SchemaObject | SchemaUnion> = {};
 
   const buildNameMap = <T extends { name: string }>(
@@ -72,21 +84,13 @@ export const buildClientSchema = ({
     }
   };
 
-  for (let i = 0; i < __schema.types.length; i++) {
-    const type = __schema.types[i];
-    if (type && type.name) {
-      const out = buildType(type);
-      if (out) typemap[type.name] = out;
-    }
-  }
-
-  return {
+  const schema: SchemaIntrospector = {
     query: __schema.queryType ? __schema.queryType.name : null,
     mutation: __schema.mutationType ? __schema.mutationType.name : null,
     subscription: __schema.subscriptionType
       ? __schema.subscriptionType.name
       : null,
-    types: typemap,
+    types: undefined,
     isSubType(abstract: string, possible: string) {
       const abstractType = typemap[abstract];
       const possibleType = typemap[possible];
@@ -104,4 +108,17 @@ export const buildClientSchema = ({
       }
     },
   };
+
+  if (__schema.types) {
+    schema.types = typemap;
+    for (let i = 0; i < __schema.types.length; i++) {
+      const type = __schema.types[i];
+      if (type && type.name) {
+        const out = buildType(type);
+        if (out) typemap[type.name] = out;
+      }
+    }
+  }
+
+  return schema;
 };
diff --git a/exchanges/graphcache/src/ast/schemaPredicates.ts b/exchanges/graphcache/src/ast/schemaPredicates.ts
index 8075ccc6..b7c27c1a 100644
--- a/exchanges/graphcache/src/ast/schemaPredicates.ts
+++ b/exchanges/graphcache/src/ast/schemaPredicates.ts
@@ -53,8 +53,8 @@ export const isInterfaceOfType = (
   const typeCondition = getTypeCondition(node);
   if (!typeCondition || typename === typeCondition) return true;
   if (
-    schema.types[typeCondition] &&
-    schema.types[typeCondition].kind === 'OBJECT'
+    schema.types![typeCondition] &&
+    schema.types![typeCondition].kind === 'OBJECT'
   )
     return typeCondition === typename;
   expectAbstractType(schema, typeCondition!);
@@ -68,7 +68,7 @@ const getField = (
   fieldName: string
 ) => {
   expectObjectType(schema, typename);
-  const object = schema.types[typename] as SchemaObject;
+  const object = schema.types![typename] as SchemaObject;
   const field = object.fields[fieldName];
   if (!field) {
     warn(
@@ -88,7 +88,7 @@ const getField = (
 
 function expectObjectType(schema: SchemaIntrospector, typename: string) {
   invariant(
-    schema.types[typename] && schema.types[typename].kind === 'OBJECT',
+    schema.types![typename] && schema.types![typename].kind === 'OBJECT',
     'Invalid Object type: The type `' +
       typename +
       '` is not an object in the defined schema, ' +
@@ -99,9 +99,9 @@ function expectObjectType(schema: SchemaIntrospector, typename: string) {
 
 function expectAbstractType(schema: SchemaIntrospector, typename: string) {
   invariant(
-    schema.types[typename] &&
-      (schema.types[typename].kind === 'INTERFACE' ||
-        schema.types[typename].kind === 'UNION'),
+    schema.types![typename] &&
+      (schema.types![typename].kind === 'INTERFACE' ||
+        schema.types![typename].kind === 'UNION'),
     'Invalid Abstract type: The type `' +
       typename +
       '` is not an Interface or Union type in the defined schema, ' +
@@ -116,7 +116,7 @@ export function expectValidKeyingConfig(
 ): void {
   if (process.env.NODE_ENV !== 'production') {
     for (const key in keys) {
-      if (!schema.types[key]) {
+      if (!schema.types![key]) {
         warn(
           'Invalid Object type: The type `' +
             key +
@@ -137,7 +137,7 @@ export function expectValidUpdatesConfig(
   }
 
   if (schema.mutation) {
-    const mutationFields = (schema.types[schema.mutation] as SchemaObject)
+    const mutationFields = (schema.types![schema.mutation] as SchemaObject)
       .fields;
     const givenMutations = updates[schema.mutation] || {};
     for (const fieldName in givenMutations) {
@@ -153,7 +153,7 @@ export function expectValidUpdatesConfig(
   }
 
   if (schema.subscription) {
-    const subscriptionFields = (schema.types[
+    const subscriptionFields = (schema.types![
       schema.subscription
     ] as SchemaObject).fields;
     const givenSubscription = updates[schema.subscription] || {};
@@ -200,7 +200,7 @@ export function expectValidResolversConfig(
   for (const key in resolvers) {
     if (key === 'Query') {
       if (schema.query) {
-        const validQueries = (schema.types[schema.query] as SchemaObject)
+        const validQueries = (schema.types![schema.query] as SchemaObject)
           .fields;
         for (const resolverQuery in resolvers.Query) {
           if (!validQueries[resolverQuery]) {
@@ -211,18 +211,18 @@ export function expectValidResolversConfig(
         warnAboutResolver('Query');
       }
     } else {
-      if (!schema.types[key]) {
+      if (!schema.types![key]) {
         warnAboutResolver(key);
       } else if (
-        schema.types[key].kind === 'INTERFACE' ||
-        schema.types[key].kind === 'UNION'
+        schema.types![key].kind === 'INTERFACE' ||
+        schema.types![key].kind === 'UNION'
       ) {
         warnAboutAbstractResolver(
           key,
-          schema.types[key].kind as 'INTERFACE' | 'UNION'
+          schema.types![key].kind as 'INTERFACE' | 'UNION'
         );
       } else {
-        const validTypeProperties = (schema.types[key] as SchemaObject).fields;
+        const validTypeProperties = (schema.types![key] as SchemaObject).fields;
         for (const resolverProperty in resolvers[key]) {
           if (!validTypeProperties[resolverProperty]) {
             warnAboutResolver(key + '.' + resolverProperty);
@@ -242,7 +242,7 @@ export function expectValidOptimisticMutationsConfig(
   }
 
   if (schema.mutation) {
-    const validMutations = (schema.types[schema.mutation] as SchemaObject)
+    const validMutations = (schema.types![schema.mutation] as SchemaObject)
       .fields;
     for (const mutation in optimisticMutations) {
       if (!validMutations[mutation]) {
diff --git a/exchanges/graphcache/src/cacheExchange.ts b/exchanges/graphcache/src/cacheExchange.ts
index 3773e933..36586384 100644
--- a/exchanges/graphcache/src/cacheExchange.ts
+++ b/exchanges/graphcache/src/cacheExchange.ts
@@ -1,5 +1,3 @@
-import { IntrospectionQuery } from 'graphql';
-
 import {
   Exchange,
   formatDocument,
@@ -30,7 +28,7 @@ import {
 import { query, write, writeOptimistic } from './operations';
 import { makeDict, isDictEmpty } from './helpers/dict';
 import { addCacheOutcome, toRequestPolicy } from './helpers/operation';
-import { filterVariables, getMainOperation } from './ast';
+import { IntrospectionData, filterVariables, getMainOperation } from './ast';
 import { Store, noopDataState, hydrateData, reserveLayer } from './store';
 
 import {
@@ -57,7 +55,7 @@ export interface CacheExchangeOpts {
   resolvers?: ResolverConfig;
   optimistic?: OptimisticMutationConfig;
   keys?: KeyingConfig;
-  schema?: IntrospectionQuery;
+  schema?: IntrospectionData;
   storage?: StorageAdapter;
 }
 
diff --git a/exchanges/graphcache/src/offlineExchange.ts b/exchanges/graphcache/src/offlineExchange.ts
index a3596212..20406e6f 100644
--- a/exchanges/graphcache/src/offlineExchange.ts
+++ b/exchanges/graphcache/src/offlineExchange.ts
@@ -20,12 +20,13 @@ import {
   getName,
 } from './ast';
 
-import { makeDict } from './helpers/dict';
 import {
   SerializedRequest,
   OptimisticMutationConfig,
   Variables,
 } from './types';
+
+import { makeDict } from './helpers/dict';
 import { cacheExchange, CacheExchangeOpts } from './cacheExchange';
 import { toRequestPolicy } from './helpers/operation';
 
diff --git a/exchanges/graphcache/src/store/store.ts b/exchanges/graphcache/src/store/store.ts
index 6e5059cf..cacea7d7 100644
--- a/exchanges/graphcache/src/store/store.ts
+++ b/exchanges/graphcache/src/store/store.ts
@@ -1,5 +1,4 @@
-import { DocumentNode, IntrospectionQuery } from 'graphql';
-
+import { DocumentNode } from 'graphql';
 import { TypedDocumentNode, formatDocument, createRequest } from '@urql/core';
 
 import {
@@ -25,6 +24,7 @@ import { keyOfField } from './keys';
 import * as InMemoryData from './data';
 
 import {
+  IntrospectionData,
   SchemaIntrospector,
   buildClientSchema,
   expectValidKeyingConfig,
@@ -40,7 +40,7 @@ export interface StoreOpts {
   resolvers?: ResolverConfig;
   optimistic?: OptimisticMutationConfig;
   keys?: KeyingConfig;
-  schema?: IntrospectionQuery;
+  schema?: IntrospectionData;
 }
 
 export class Store implements Cache {
@@ -66,10 +66,12 @@ export class Store implements Cache {
     let mutationName = 'Mutation';
     let subscriptionName = 'Subscription';
     if (opts.schema) {
-      const schema = (this.schema = buildClientSchema(opts.schema));
+      const schema = buildClientSchema(opts.schema);
       queryName = schema.query || queryName;
       mutationName = schema.mutation || mutationName;
       subscriptionName = schema.subscription || subscriptionName;
+      // Only add schema introspector if it has types info
+      if (schema.types) this.schema = schema;
     }
 
     this.updates = {
diff --git a/exchanges/request-policy/src/requestPolicyExchange.ts b/exchanges/request-policy/src/requestPolicyExchange.ts
index 7078fab0..5e211e86 100644
--- a/exchanges/request-policy/src/requestPolicyExchange.ts
+++ b/exchanges/request-policy/src/requestPolicyExchange.ts
@@ -1,5 +1,10 @@
-import { makeOperation, Operation, Exchange } from '@urql/core';
-import { pipe, map } from 'wonka';
+import {
+  makeOperation,
+  Operation,
+  OperationResult,
+  Exchange,
+} from '@urql/core';
+import { pipe, tap, map } from 'wonka';
 
 const defaultTTL = 5 * 60 * 1000;
 
@@ -12,27 +17,25 @@ export const requestPolicyExchange = (options: Options): Exchange => ({
   forward,
 }) => {
   const operations = new Map();
-
   const TTL = (options || {}).ttl || defaultTTL;
 
   const processIncomingOperation = (operation: Operation): Operation => {
     if (
-      operation.context.requestPolicy !== 'cache-first' &&
-      operation.context.requestPolicy !== 'cache-only'
-    )
-      return operation;
-    if (!operations.has(operation.key)) {
-      operations.set(operation.key, new Date());
+      operation.kind !== 'query' ||
+      (operation.context.requestPolicy !== 'cache-first' &&
+        operation.context.requestPolicy !== 'cache-only') ||
+      !operations.has(operation.key)
+    ) {
       return operation;
     }
 
     const lastOccurrence = operations.get(operation.key);
     const currentTime = new Date().getTime();
     if (
-      currentTime - lastOccurrence.getTime() > TTL &&
+      currentTime - lastOccurrence > TTL &&
       (options.shouldUpgrade ? options.shouldUpgrade(operation) : true)
     ) {
-      operations.set(operation.key, new Date());
+      operations.delete(operation.key);
       return makeOperation(operation.kind, operation, {
         ...operation.context,
         requestPolicy: 'cache-and-network',
@@ -42,7 +45,14 @@ export const requestPolicyExchange = (options: Options): Exchange => ({
     return operation;
   };
 
+  const processIncomingResults = (result: OperationResult): void => {
+    operations.set(result.operation.key, new Date().getTime());
+  };
+
   return ops$ => {
-    return forward(pipe(ops$, map(processIncomingOperation)));
+    return pipe(
+      forward(pipe(ops$, map(processIncomingOperation))),
+      tap(processIncomingResults)
+    );
   };
 };
diff --git a/packages/react-urql/src/hooks/useQuery.ts b/packages/react-urql/src/hooks/useQuery.ts
index ac93044f..8193cbcc 100644
--- a/packages/react-urql/src/hooks/useQuery.ts
+++ b/packages/react-urql/src/hooks/useQuery.ts
@@ -1,3 +1,5 @@
+// eslint-disable react-hooks/exhaustive-deps
+
 import { DocumentNode } from 'graphql';
 import { Source, pipe, subscribe, onEnd, onPush, takeWhile } from 'wonka';
 import { useState, useEffect, useCallback, useMemo } from 'react';
diff --git a/packages/react-urql/src/hooks/useSubscription.ts b/packages/react-urql/src/hooks/useSubscription.ts
index 55de2557..447075eb 100644
--- a/packages/react-urql/src/hooks/useSubscription.ts
+++ b/packages/react-urql/src/hooks/useSubscription.ts
@@ -1,3 +1,5 @@
+// eslint-disable react-hooks/exhaustive-deps
+
 import { DocumentNode } from 'graphql';
 import { pipe, subscribe, onEnd } from 'wonka';
 import { useEffect, useState, useCallback, useMemo } from 'react';


Test patch:
diff --git a/exchanges/graphcache/src/store/store.test.ts b/exchanges/graphcache/src/store/store.test.ts
index d057819c..e155fc95 100644
--- a/exchanges/graphcache/src/store/store.test.ts
+++ b/exchanges/graphcache/src/store/store.test.ts
@@ -881,9 +881,19 @@ describe('Store with storage', () => {
     const fakeUpdater = jest.fn();
 
     const store = new Store({
-      schema: minifyIntrospectionQuery(
-        require('../test-utils/altered_root_schema.json')
-      ),
+      schema: {
+        __schema: {
+          queryType: {
+            name: 'query_root',
+          },
+          mutationType: {
+            name: 'mutation_root',
+          },
+          subscriptionType: {
+            name: 'subscription_root',
+          },
+        },
+      },
       updates: {
         Mutation: {
           toggleTodo: fakeUpdater,