Fix Scala codegen emitting fields under names that don't match the schema (#1008)

shadaj · trevor-scheer · commit 12cdd3c35cd5 · 2019-02-13T00:14:50.000-08:00
* Fix Scala codegen emitting fields under names that don't match the schema

* Improve typing in code generator
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,8 @@
 - `vscode-apollo@1.5.0`
   - Fix inline graphql highlighting in Vue `<script>` tags [#981](https://github.com/apollographql/apollo-tooling/pull/981)
   - Fix graphql comments not being highlighted correctly [#907](https://github.com/apollographql/apollo-tooling/pull/907)
+- `apollo-codegen-scala`
+  - Fix types sometimes being emitted with fields that don't match the underlying data [#1008](https://github.com/apollographql/apollo-tooling/pull/1008)
 
 ## `apollo@2.4.4`, `apollo-codegen-scala@0.33.0`
 
diff --git a/packages/apollo-codegen-scala/src/__tests__/__snapshots__/codeGeneration.ts.snap b/packages/apollo-codegen-scala/src/__tests__/__snapshots__/codeGeneration.ts.snap
diff --git a/packages/apollo-codegen-scala/src/__tests__/codeGeneration.ts b/packages/apollo-codegen-scala/src/__tests__/codeGeneration.ts
@@ -453,6 +453,38 @@ describe("Scala code generation", function() {
       expect(generator.output).toMatchSnapshot();
     });
 
+    test(`should handle underscores in a trait declaration for a selection set`, function() {
+      traitDeclarationForSelectionSet(generator, {
+        traitName: "TypeWithUnderscore",
+        parentType: undefined,
+        fields: [
+          {
+            responseName: "_id",
+            fieldName: "_id",
+            type: GraphQLString
+          }
+        ]
+      });
+
+      expect(generator.output).toMatchSnapshot();
+    });
+
+    test(`should handle escaped values in a trait declaration for a selection set`, function() {
+      traitDeclarationForSelectionSet(generator, {
+        traitName: "TypeWithUnderscore",
+        parentType: undefined,
+        fields: [
+          {
+            responseName: "class",
+            fieldName: "class",
+            type: GraphQLString
+          }
+        ]
+      });
+
+      expect(generator.output).toMatchSnapshot();
+    });
+
     test(`should generate a nested trait declaration for a selection set with subselections`, function() {
       traitDeclarationForSelectionSet(generator, {
         traitName: "Hero",
diff --git a/packages/apollo-codegen-scala/src/codeGeneration.ts b/packages/apollo-codegen-scala/src/codeGeneration.ts
@@ -443,7 +443,10 @@ function dataContainerDeclaration(
     insideCompanion
   }: {
     name: string;
-    properties: Property[];
+    properties: (Property & {
+      name?: string;
+      responseName?: string;
+    })[];
     extraSuperClasses?: string[];
     description?: string;
     insideCompanion?: () => void;
@@ -460,6 +463,7 @@ function dataContainerDeclaration(
     () => {
       properties.forEach(p => {
         propertyDeclaration(generator, {
+          jsName: p.name || p.responseName,
           propertyName: p.propertyName,
           typeName: p.typeName
         });
@@ -489,7 +493,7 @@ function dataContainerDeclaration(
         },
         () => {
           const propertiesIn = properties
-            .map(p => `${p.propertyName} = ${p.propertyName}`)
+            .map(p => `"${p.name || p.responseName}" -> ${p.propertyName}`)
             .join(", ");
           generator.printOnNewline(
             `scala.scalajs.js.Dynamic.literal(${propertiesIn}).asInstanceOf[${name}]`
@@ -536,7 +540,7 @@ function dataContainerDeclaration(
           },
           () => {
             const propertiesIn = properties
-              .map(p => `${p.propertyName} = ${p.propertyName}`)
+              .map(p => `"${p.name || p.responseName}" -> ${p.propertyName}`)
               .join(", ");
             generator.printOnNewline(
               `scala.scalajs.js.Dynamic.literal(${propertiesIn}).asInstanceOf[${name}]`
diff --git a/packages/apollo-codegen-scala/src/language.ts b/packages/apollo-codegen-scala/src/language.ts
@@ -130,10 +130,12 @@ export function methodDeclaration(
 export function propertyDeclaration(
   generator: CodeGenerator<LegacyCompilerContext, any>,
   {
+    jsName,
     propertyName,
     typeName,
     description
   }: {
+    jsName?: string;
     propertyName: string;
     typeName: string;
     description?: string;
@@ -145,7 +147,9 @@ export function propertyDeclaration(
   }
 
   generator.printOnNewline(
-    `val ${propertyName}: ${typeName}` + (closure ? ` =` : "")
+    (jsName ? `@scala.scalajs.js.annotation.JSName("${jsName}") ` : "") +
+      `val ${propertyName}: ${typeName}` +
+      (closure ? ` =` : "")
   );
 
   if (closure) {
diff --git a/packages/apollo-codegen-scala/src/naming.ts b/packages/apollo-codegen-scala/src/naming.ts
@@ -57,47 +57,31 @@ export function propertyFromInputField(
   const isOptional = !(type instanceof GraphQLNonNull);
   const bareType = getNamedType(type);
 
-  if (isCompositeType(bareType)) {
-    const bareTypeName = join(
-      [
-        namespace,
-        parentTraitName,
-        escapeIdentifierIfNeeded(pascalCase(Inflector.singularize(name)))
-      ],
-      "."
-    );
-    const typeName = typeNameFromGraphQLType(
-      context,
-      type,
-      bareTypeName,
-      isOptional,
-      true
-    );
-    return {
-      ...field,
-      propertyName,
-      typeName,
-      isOptional,
-      isList,
-      description: field.description || undefined
-    };
-  } else {
-    const typeName = typeNameFromGraphQLType(
-      context,
-      type,
-      undefined,
-      isOptional,
-      true
-    );
-    return {
-      ...field,
-      propertyName,
-      typeName,
-      isOptional,
-      isList,
-      description: field.description || undefined
-    };
-  }
+  const bareTypeName = isCompositeType(bareType)
+    ? join(
+        [
+          namespace,
+          parentTraitName,
+          escapeIdentifierIfNeeded(pascalCase(Inflector.singularize(name)))
+        ],
+        "."
+      )
+    : undefined;
+  const typeName = typeNameFromGraphQLType(
+    context,
+    type,
+    bareTypeName,
+    isOptional,
+    true
+  );
+  return {
+    ...field,
+    propertyName,
+    typeName,
+    isOptional,
+    isList,
+    description: field.description || undefined
+  };
 }
 
 export function propertyFromLegacyField(
@@ -107,39 +91,30 @@ export function propertyFromLegacyField(
   parentTraitName?: string
 ): LegacyField & Property {
   const name = field.responseName;
-  const unescapedPropertyName = isMetaFieldName(name) ? name : camelCase(name);
-  const propertyName = escapeIdentifierIfNeeded(unescapedPropertyName);
+  const propertyName = escapeIdentifierIfNeeded(name);
 
   const type = field.type;
   const isList = type instanceof GraphQLList || type instanceof GraphQLList;
   const isOptional = field.isConditional || !(type instanceof GraphQLNonNull);
   const bareType = getNamedType(type);
 
-  if (isCompositeType(bareType)) {
-    const bareTypeName = join(
-      [
-        namespace,
-        parentTraitName,
-        escapeIdentifierIfNeeded(pascalCase(Inflector.singularize(name)))
-      ],
-      "."
-    );
-    const typeName = typeNameFromGraphQLType(
-      context,
-      type,
-      bareTypeName,
-      isOptional
-    );
-    return { ...field, propertyName, typeName, isOptional, isList };
-  } else {
-    const typeName = typeNameFromGraphQLType(
-      context,
-      type,
-      undefined,
-      isOptional
-    );
-    return { ...field, propertyName, typeName, isOptional, isList };
-  }
+  const bareTypeName = isCompositeType(bareType)
+    ? join(
+        [
+          namespace,
+          parentTraitName,
+          escapeIdentifierIfNeeded(pascalCase(Inflector.singularize(name)))
+        ],
+        "."
+      )
+    : undefined;
+  const typeName = typeNameFromGraphQLType(
+    context,
+    type,
+    bareTypeName,
+    isOptional
+  );
+  return { ...field, propertyName, typeName, isOptional, isList };
 }
 
 function isMetaFieldName(name: string) {
diff --git a/packages/apollo/src/commands/client/__tests__/__snapshots__/generate.test.ts.snap b/packages/apollo/src/commands/client/__tests__/__snapshots__/generate.test.ts.snap
@@ -283,12 +283,12 @@ object SimpleQueryQuery extends com.apollographql.scalajs.GraphQLQuery {
   type Variables = Unit
 
   @scala.scalajs.js.native trait Data extends scala.scalajs.js.Object {
-    val hello: String
+    @scala.scalajs.js.annotation.JSName(\\"hello\\") val hello: String
   }
 
   object Data {
     def apply(hello: String) = {
-      scala.scalajs.js.Dynamic.literal(hello = hello).asInstanceOf[Data]
+      scala.scalajs.js.Dynamic.literal(\\"hello\\" -> hello).asInstanceOf[Data]
     }
 
     def unapply(value: Data) = {
@@ -297,7 +297,7 @@ object SimpleQueryQuery extends com.apollographql.scalajs.GraphQLQuery {
 
     implicit class CopyExtensions(private val orig: Data) extends AnyVal {
       def copy(hello: String = orig.hello) = {
-        scala.scalajs.js.Dynamic.literal(hello = hello).asInstanceOf[Data]
+        scala.scalajs.js.Dynamic.literal(\\"hello\\" -> hello).asInstanceOf[Data]
       }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -283,12 +283,12 @@ object SimpleQueryQuery extends com.apollographql.scalajs.GraphQLQuery {`
`283`	`283`	`type Variables = Unit`
`284`	`284`
`285`	`285`	`@scala.scalajs.js.native trait Data extends scala.scalajs.js.Object {`
`286`		`- val hello: String`
	`286`	`+ @scala.scalajs.js.annotation.JSName(\\"hello\\") val hello: String`
`287`	`287`	`}`
`288`	`288`
`289`	`289`	`object Data {`
`290`	`290`	`def apply(hello: String) = {`
`291`		`- scala.scalajs.js.Dynamic.literal(hello = hello).asInstanceOf[Data]`
	`291`	`+ scala.scalajs.js.Dynamic.literal(\\"hello\\" -> hello).asInstanceOf[Data]`
`292`	`292`	`}`
`293`	`293`
`294`	`294`	`def unapply(value: Data) = {`
`@@ -297,7 +297,7 @@ object SimpleQueryQuery extends com.apollographql.scalajs.GraphQLQuery {`
`297`	`297`
`298`	`298`	`implicit class CopyExtensions(private val orig: Data) extends AnyVal {`
`299`	`299`	`def copy(hello: String = orig.hello) = {`
`300`		`- scala.scalajs.js.Dynamic.literal(hello = hello).asInstanceOf[Data]`
	`300`	`+ scala.scalajs.js.Dynamic.literal(\\"hello\\" -> hello).asInstanceOf[Data]`
`301`	`301`	`}`
`302`	`302`	`}`
`303`	`303`