optimizing requestid (#2985)

Summary: #2984 Modified the compilation of the artifacts in order to generate a unique identifier (md5 text) for the 'Request' nodes. modified writeRelayGeneratedFile: eliminate the switch, the node is always of type Request because inside 'if (generatedNode.kind === RelayConcreteNode.REQUEST) {' https://github.com/facebook/relay/blob/master/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L98 Pull Request resolved: #2985 Reviewed By: josephsavona Differential Revision: D19373941 Pulled By: kassens fbshipit-source-id: 3bb7ada15fb6ee781ba1838e806294d74de6afe7
facebook · Jun 11, 2020 · a3acc08 · a3acc08
1 parent 08bb2d2
commit a3acc08
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 91 deletions.
diff --git a/packages/react-relay/ReactRelayQueryRenderer.js b/packages/react-relay/ReactRelayQueryRenderer.js
@@ -320,9 +320,8 @@ function getRequestCacheKey(
   request: RequestParameters,
   variables: Variables,
 ): string {
-  const requestID = request.id || request.text;
   return JSON.stringify({
-    id: String(requestID),
+    id: request.cacheID ? request.cacheID : request.id,
     variables,
   });
 }

diff --git a/packages/relay-compiler/codegen/RelayCodeGenerator.js b/packages/relay-compiler/codegen/RelayCodeGenerator.js
@@ -14,9 +14,9 @@
 
 const NormalizationCodeGenerator = require('./NormalizationCodeGenerator');
 const ReaderCodeGenerator = require('./ReaderCodeGenerator');
-
 const sortObjectByKey = require('./sortObjectByKey');
-
+const md5 = require('../util/md5');
+const nullthrows = require('nullthrows');
 const {createCompilerError} = require('../core/CompilerError');
 
 import type {Fragment, Request, SplitOperation} from '../core/IR';
@@ -57,13 +57,21 @@ function generate(
         fragment: ReaderCodeGenerator.generate(schema, node.fragment),
         kind: 'Request',
         operation: NormalizationCodeGenerator.generate(schema, node.root),
-        params: {
-          id: node.id,
-          metadata: sortObjectByKey(node.metadata),
-          name: node.name,
-          operationKind: node.root.operation,
-          text: node.text,
-        },
+        params:
+          node.id != null
+            ? {
+                id: node.id,
+                metadata: sortObjectByKey(node.metadata),
+                name: node.name,
+                operationKind: node.root.operation,
+              }
+            : {
+                cacheID: md5(nullthrows(node.text)),
+                metadata: sortObjectByKey(node.metadata),
+                name: node.name,
+                operationKind: node.root.operation,
+                text: node.text,
+              },
       };
     case 'SplitOperation':
       return NormalizationCodeGenerator.generate(schema, node);

diff --git a/packages/relay-compiler/codegen/writeRelayGeneratedFile.js b/packages/relay-compiler/codegen/writeRelayGeneratedFile.js
@@ -89,70 +89,68 @@ function writeRelayGeneratedFile(
 
   let docText;
   if (generatedNode.kind === RelayConcreteNode.REQUEST) {
-    docText = generatedNode.params.text;
+    docText =
+      generatedNode.params.text != null ? generatedNode.params.text : null;
   }
 
   // Use `Promise.resolve` to work around a Babel 7.8/7.9 issue.
   return Promise.resolve().then(async () => {
     let hash = null;
-    if (generatedNode.kind === RelayConcreteNode.REQUEST && persistQuery) {
-      const {text} = generatedNode.params;
+    if (generatedNode.kind === RelayConcreteNode.REQUEST) {
       invariant(
-        text != null,
-        'writeRelayGeneratedFile: Expected `text` in order to persist query',
+        docText != null,
+        'writeRelayGeneratedFile: Expected `text` for operations to be set.',
       );
 
-      hash = md5(text);
-
-      let id = null;
-      if (!shouldRepersist) {
-        // Unless we `shouldRepersist` the query, check if the @relayHash matches
-        // the operation text of the current text and re-use the persisted
-        // operation id.
-        const oldContent = codegenDir.read(filename);
-        const oldHash = extractHash(oldContent);
-        const oldRequestID = extractRelayRequestID(oldContent);
+      const {
+        isRefetchableQuery: _ignored,
+        derivedFrom: _ignored2,
+        ...nextMetadata
+      } = generatedNode.params.metadata;
 
-        if (hash === oldHash && oldRequestID != null) {
-          id = oldRequestID;
+      let nextRequestParams;
+      if (persistQuery != null) {
+        hash = md5(docText);
+
+        let id = null;
+        if (!shouldRepersist) {
+          // Unless we `shouldRepersist` the query, check if the @relayHash matches
+          // the operation text of the current text and re-use the persisted
+          // operation id.
+          const oldContent = codegenDir.read(filename);
+          const oldHash = extractHash(oldContent);
+          const oldRequestID = extractRelayRequestID(oldContent);
+          if (hash === oldHash && oldRequestID != null) {
+            id = oldRequestID;
+          }
         }
-      }
-      if (id == null) {
-        id = await persistQuery(text);
-      }
-
-      generatedNode = {
-        ...generatedNode,
-        params: {
+        if (id == null) {
+          id = await persistQuery(docText);
+        }
+        nextRequestParams = {
           id,
-          metadata: generatedNode.params.metadata,
+          metadata: nextMetadata,
           name: generatedNode.params.name,
           operationKind: generatedNode.params.operationKind,
           text: null,
-        },
-      };
-    }
-
-    // Strip metadata only used within the compiler
-    if (
-      generatedNode.kind === RelayConcreteNode.REQUEST &&
-      (generatedNode.params.metadata?.isRefetchableQuery != null ||
-        generatedNode.params.metadata?.derivedFrom != null)
-    ) {
-      const {
-        derivedFrom: _ignored,
-        isRefetchableQuery: _ignored2,
-        ...metadata
-      } = generatedNode.params.metadata;
+        };
+      } else {
+        nextRequestParams = {
+          cacheID: md5(docText),
+          id: null,
+          metadata: nextMetadata,
+          name: generatedNode.params.name,
+          operationKind: generatedNode.params.operationKind,
+          text: docText,
+        };
+      }
       generatedNode = {
         ...generatedNode,
-        // $FlowFixMe
-        params: {
-          ...generatedNode.params,
-          metadata,
-        },
+        params: nextRequestParams,
       };
     }
+
+    // Strip metadata only used within the compiler
     if (
       generatedNode.kind === RelayConcreteNode.SPLIT_OPERATION &&
       generatedNode.metadata?.derivedFrom != null

diff --git a/packages/relay-runtime/util/RelayConcreteNode.js b/packages/relay-runtime/util/RelayConcreteNode.js
@@ -19,10 +19,10 @@ import type {
 import type {ReaderFragment, ReaderInlineDataFragment} from './ReaderNode';
 
 /**
- * Represents a common GraphQL request with `text` (or persisted `id`) can be
- * used to execute it, an `operation` containing information to normalize the
- * results, and a `fragment` derived from that operation to read the response
- * data (masking data from child fragments).
+ * Represents a common GraphQL request that can be executed, an `operation`
+ * containing information to normalize the results, and a `fragment` derived
+ * from that operation to read the response data (masking data from child
+ * fragments).
  */
 export type ConcreteRequest = {|
   +kind: 'Request',
@@ -32,17 +32,29 @@ export type ConcreteRequest = {|
 |};
 
 /**
- * Contains the `text` (or persisted `id`) required for executing a common
- * GraphQL request.
+ * Contains the parameters required for executing a GraphQL request.
+ * The operation can either be provided as a persisted `id` or `text`. If given
+ * in `text` format, a `cacheID` as a hash of the text should be set to be used
+ * for local caching.
  */
 export type RequestParameters =
-  | {|...BaseRequestParameters, +text: null, +id: string|}
-  | {|...BaseRequestParameters, +text: string, +id: null|};
-type BaseRequestParameters = {|
-  +name: string,
-  +operationKind: 'mutation' | 'query' | 'subscription',
-  +metadata: {[key: string]: mixed, ...},
-|};
+  | {|
+      +id: string,
+      +text: null,
+      // common fields
+      +name: string,
+      +operationKind: 'mutation' | 'query' | 'subscription',
+      +metadata: {[key: string]: mixed, ...},
+    |}
+  | {|
+      +cacheID: string,
+      +id: null,
+      +text: string,
+      // common fields
+      +name: string,
+      +operationKind: 'mutation' | 'query' | 'subscription',
+      +metadata: {[key: string]: mixed, ...},
+    |};
 
 export type GeneratedNode =
   | ConcreteRequest

diff --git a/packages/relay-runtime/util/__tests__/getRequestIdentifier-test.js b/packages/relay-runtime/util/__tests__/getRequestIdentifier-test.js
@@ -15,13 +15,18 @@
 
 const getRequestIdentifier = require('../getRequestIdentifier');
 
+import type {RequestParameters} from '../../util/RelayConcreteNode';
+
 describe('getRequestIdentifier', () => {
   it('passes with `id`', () => {
     const queryIdentifier = getRequestIdentifier(
       ({
         name: 'FooQuery',
+        operationKind: 'query',
+        metadata: {},
         id: '123',
-      }: any),
+        text: null,
+      }: RequestParameters),
       {foo: 1},
     );
     expect(queryIdentifier).toEqual('123{"foo":1}');
@@ -31,24 +36,14 @@ describe('getRequestIdentifier', () => {
     const queryIdentifier = getRequestIdentifier(
       ({
         name: 'FooQuery',
-        text: 'query FooQuery {}',
-      }: any),
-      {bar: 1},
-    );
-    expect(queryIdentifier).toEqual('query FooQuery {}{"bar":1}');
-  });
-
-  it('fails without `id` or `text`', () => {
-    expect(() => {
-      getRequestIdentifier(
-        ({
-          name: 'FooQuery',
-        }: any),
-        {foo: 1},
-      );
-    }).toThrowError(
-      'getRequestIdentifier: Expected request `FooQuery` to have ' +
-        'either a valid `id` or `text` property',
+        operationKind: 'query',
+        metadata: {},
+        id: null,
+        text: 'query Test { __typename }',
+        cacheID: 'test-cache-id',
+      }: RequestParameters),
+      {foo: 1},
     );
+    expect(queryIdentifier).toEqual('test-cache-id{"foo":1}');
   });
 });
diff --git a/packages/relay-runtime/util/getRequestIdentifier.js b/packages/relay-runtime/util/getRequestIdentifier.js
@@ -28,11 +28,12 @@ function getRequestIdentifier(
   parameters: RequestParameters,
   variables: Variables,
 ): RequestIdentifier {
-  const requestID = parameters.id != null ? parameters.id : parameters.text;
+  const requestID =
+    parameters.cacheID != null ? parameters.cacheID : parameters.id;
   invariant(
     requestID != null,
     'getRequestIdentifier: Expected request `%s` to have either a ' +
-      'valid `id` or `text` property',
+      'valid `id` or `cacheID` property',
     parameters.name,
   );
   return requestID + JSON.stringify(stableCopy(variables));