Fix issue with Relay serialization (#2638)

Summary: Adds support for schemas defined directly with graphql-js, where enums can have an alternative runtime representation (ie, printing as a string but internally represented as an int). -- original pr -- Fixes #2633. I had to add dependency to actually add enums with proper test values, because monolithic SDL-first is so limited. Pull Request resolved: #2638 Reviewed By: alunyov Differential Revision: D14066748 Pulled By: josephsavona fbshipit-source-id: 811c591c029ffa175d32aa564e616a10b0541f30
facebook · Feb 14, 2019 · 8f4e829 · 8f4e829
1 parent 13493a9
commit 8f4e829
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 19 deletions.
diff --git a/packages/relay-compiler/core/GraphQLIRPrinter.js b/packages/relay-compiler/core/GraphQLIRPrinter.js
@@ -15,9 +15,11 @@ const invariant = require('invariant');
 const {DEFAULT_HANDLE_KEY} = require('../util/DefaultHandleKey');
 const {
   GraphQLEnumType,
+  GraphQLID,
   GraphQLInputObjectType,
   GraphQLList,
   GraphQLNonNull,
+  GraphQLScalarType,
 } = require('graphql');
 
 import type {CompilerContextDocument} from './GraphQLCompilerContext';
@@ -297,19 +299,34 @@ function printValue(value: ArgumentValue, type: ?GraphQLInputType): ?string {
 }
 
 function printLiteral(value: mixed, type: ?GraphQLInputType): string {
+  if (value == null) {
+    return JSON.stringify(value);
+  }
   if (type instanceof GraphQLNonNull) {
     type = type.ofType;
   }
   if (type instanceof GraphQLEnumType) {
+    let result = type.serialize(value);
+    if (result == null && typeof value === 'string') {
+      // For backwards compatibility, print invalid input values as-is. This
+      // can occur with literals defined as an @argumentDefinitions
+      // defaultValue.
+      result = value;
+    }
     invariant(
-      typeof value === 'string',
-      'GraphQLIRPrinter: Expected value of type %s to be a string, got `%s`.',
+      typeof result === 'string',
+      'GraphQLIRPrinter: Expected value of type %s to be a valid enum value, got `%s`.',
       type.name,
-      value,
+      JSON.stringify(value),
     );
-    return value;
-  }
-  if (Array.isArray(value)) {
+    return result;
+  } else if (type === GraphQLID) {
+    // For backwards compatibility, print ID values as-is
+    return JSON.stringify(value);
+  } else if (type instanceof GraphQLScalarType) {
+    const result = type.serialize(value);
+    return JSON.stringify(result);
+  } else if (Array.isArray(value)) {
     invariant(
       type instanceof GraphQLList,
       'GraphQLIRPrinter: Need a type in order to print arrays.',

diff --git a/packages/relay-compiler/core/__tests__/GraphQLIRVisitor-test.js b/packages/relay-compiler/core/__tests__/GraphQLIRVisitor-test.js
@@ -95,11 +95,23 @@ describe('GraphQLIRVisitor', () => {
           },
           Literal: {
             leave(node: Literal) {
+              const mutator = value => {
+                // Keep enums valid
+                if (value === 'WEB') {
+                  return 'MOBILE';
+                } else if (value === 'HELPFUL') {
+                  return 'DERISIVE';
+                } else if (typeof value === 'number') {
+                  return value + 10;
+                } else {
+                  return String(value) + '_mutated';
+                }
+              };
               return {
                 ...node,
                 value: Array.isArray(node.value)
-                  ? node.value.map(item => String(node.value) + '_mutated')
-                  : String(node.value) + '_mutated',
+                  ? node.value.map(mutator)
+                  : mutator(node.value),
               };
             },
           },

diff --git a/packages/relay-compiler/core/__tests__/__snapshots__/GraphQLIRVisitor-test.js.snap b/packages/relay-compiler/core/__tests__/__snapshots__/GraphQLIRVisitor-test.js.snap
@@ -79,16 +79,16 @@ fragment UserFragment_mutated on User @argumentDefinitions(
   ... on Mutated @include(if: $cond_mutated) {
     id_mutated
     __typename_mutated
-    checkins_mutated(environments_mutated: [WEB_mutated]) {
+    checkins_mutated(environments_mutated: [MOBILE]) {
       __typename_mutated
     }
-    friends_mutated(after_mutated: $after_mutated, first_mutated: $first_mutated, traits_mutated: [HELPFUL_mutated]) {
+    friends_mutated(after_mutated: $after_mutated, first_mutated: $first_mutated, traits_mutated: [DERISIVE]) {
       count_mutated
     }
     ... on Mutated @skip(if: $cond_mutated) {
       name_mutated
     }
-    thumbnail: profilePicture_mutated(size_mutated: "32_mutated") {
+    thumbnail: profilePicture_mutated(size_mutated: 42) {
       height_mutated
       width_mutated
       src: uri_mutated

diff --git a/packages/relay-compiler/core/__tests__/__snapshots__/RelayPrinter-test.js.snap b/packages/relay-compiler/core/__tests__/__snapshots__/RelayPrinter-test.js.snap
@@ -36,9 +36,18 @@ fragment UserFragment on User @argumentDefinitions(
   friends(after: $after, first: $first, traits: [HELPFUL]) {
     count
   }
+  secondFriends: friends(first: 10) {
+    count
+  }
   name @include(if: $cond)
   otherName: name @customDirective(level: 3)
-  thumbnail: profilePicture(size: 32) {
+  thumbnail: profilePicture2(
+    size: 32,
+    cropPosition: CENTER,
+    fileExtension: PNG,
+    additionalParameters: { filter: "Boston" },
+    options: {newName: null}
+  ) {
     height
     width
     src: uri
@@ -88,9 +97,12 @@ fragment UserFragment on User @argumentDefinitions(
   friends(after: $after, first: $first, traits: [HELPFUL]) {
     count
   }
+  secondFriends: friends(first: 10) {
+    count
+  }
   name @include(if: $cond)
   otherName: name @customDirective(level: 3)
-  thumbnail: profilePicture(size: 32) {
+  thumbnail: profilePicture2(size: 32, cropPosition: CENTER, fileExtension: PNG, additionalParameters: {"filter":"Boston"}, options: {newName: null}) {
     height
     width
     src: uri

diff --git a/packages/relay-compiler/core/__tests__/fixtures/printer/kitchen-sink.graphql b/packages/relay-compiler/core/__tests__/fixtures/printer/kitchen-sink.graphql
@@ -32,9 +32,18 @@ fragment UserFragment on User @argumentDefinitions(
   friends(after: $after, first: $first, traits: [HELPFUL]) {
     count
   }
+  secondFriends: friends(first: 10) {
+    count
+  }
   name @include(if: $cond)
   otherName: name @customDirective(level: 3)
-  thumbnail: profilePicture(size: 32) {
+  thumbnail: profilePicture2(
+    size: 32,
+    cropPosition: CENTER,
+    fileExtension: PNG,
+    additionalParameters: { filter: "Boston" },
+    options: {newName: null}
+  ) {
     height
     width
     src: uri

diff --git a/packages/relay-test-utils/RelayTestSchema.js b/packages/relay-test-utils/RelayTestSchema.js
@@ -14,8 +14,105 @@ const RelayTestSchemaPath = require('./RelayTestSchemaPath');
 
 const fs = require('fs');
 
-const {buildASTSchema, parse} = require('graphql');
+const {
+  parse,
+  GraphQLEnumType,
+  GraphQLSchema,
+  GraphQLScalarType,
+  Kind,
+  extendSchema,
+} = require('graphql');
 
-module.exports = buildASTSchema(
-  parse(fs.readFileSync(RelayTestSchemaPath, 'utf8'), {assumeValid: true}),
-);
+function buildSchema() {
+  const CropPosition = new GraphQLEnumType({
+    name: 'CropPosition',
+    values: {
+      TOP: {value: 1},
+      CENTER: {value: 2},
+      BOTTOM: {value: 3},
+      LEFT: {value: 4},
+      RIGHT: {value: 5},
+    },
+  });
+  const FileExtension = new GraphQLEnumType({
+    name: 'FileExtension',
+    values: {
+      JPG: {value: 'jpg'},
+      PNG: {value: 'png'},
+    },
+  });
+  let schema = new GraphQLSchema({
+    types: [CropPosition, FileExtension, GraphQLJSONType],
+  });
+  schema = extendSchema(
+    schema,
+    parse(fs.readFileSync(RelayTestSchemaPath, 'utf8')),
+  );
+  // AST Builder doesn't allow things undefined in AST to be argument types it
+  // seems
+  return extendSchema(
+    schema,
+    parse(`
+      input ProfilePictureOptions {
+        newName: String
+      }
+
+      extend type User {
+        profilePicture2(
+          size: [Int],
+          preset: PhotoSize
+          cropPosition: CropPosition
+          fileExtension: FileExtension
+          additionalParameters: JSON
+          options: ProfilePictureOptions
+        ): Image
+      }
+    `),
+  );
+}
+
+function identity(value) {
+  return value;
+}
+
+function parseLiteral(ast, variables) {
+  switch (ast.kind) {
+    case Kind.STRING:
+    case Kind.BOOLEAN:
+      return ast.value;
+    case Kind.INT:
+    case Kind.FLOAT:
+      return parseFloat(ast.value);
+    case Kind.OBJECT: {
+      const value = Object.create(null);
+      ast.fields.forEach(field => {
+        value[field.name.value] = parseLiteral(field.value, variables);
+      });
+
+      return value;
+    }
+    case Kind.LIST:
+      return ast.values.map(n => parseLiteral(n, variables));
+    case Kind.NULL:
+      return null;
+    case Kind.VARIABLE: {
+      const name = ast.name.value;
+      return variables ? variables[name] : undefined;
+    }
+    default:
+      return undefined;
+  }
+}
+
+const GraphQLJSONType = new GraphQLScalarType({
+  name: 'JSON',
+  description:
+    'The `JSON` scalar type represents JSON values as specified by ' +
+    '[ECMA-404](http://www.ecma-international.org/' +
+    'publications/files/ECMA-ST/ECMA-404.pdf).',
+  serialize: identity,
+  parseValue: identity,
+  parseLiteral,
+});
+
+module.exports = buildSchema();
diff --git a/packages/relay-test-utils/testschema.graphql b/packages/relay-test-utils/testschema.graphql
@@ -206,6 +206,14 @@ type Comment implements Node {
   message: Text
   name: String
   profilePicture(size: [Int], preset: PhotoSize): Image
+  # This is added in RelayTestSchema
+  # profilePicture2(
+  #   size: [Int],
+  #   preset: PhotoSize,
+  #   cropPosition: CropPosition,
+  #   fileExtension: FileExtension,
+  #   additionalParameters: JSON
+  # ): Image
   segments(first: Int): Segments
   screennames: [Screenname]
   subscribeStatus: String
@@ -745,7 +753,10 @@ type User implements Named & Node & Actor & HasJsField {
   nameRenderer(supported: [String!]!): UserNameRenderer
   storySearch(query: StorySearchInput): [Story]
   storyCommentSearch(query: StoryCommentSearchInput): [Comment]
-  profilePicture(size: [Int], preset: PhotoSize): Image
+  profilePicture(
+    size: [Int],
+    preset: PhotoSize
+  ): Image
   profile_picture(scale: Float): Image
   segments(first: Int): Segments
   screennames: [Screenname]
@@ -839,6 +850,20 @@ enum TopLevelCommentsOrdering {
   toplevel
 }
 
+# This is added in RelayTestSchema
+# enum CropPosition {
+#   TOP
+#   CENTER
+#   BOTTOM
+#   LEFT
+#   RIGHT
+# }
+#
+# enum FileExtension {
+#   JPG
+#   PNG
+# }
+
 type Settings {
   cache_id: ID
   notificationSounds: Boolean