Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
- `vscode-apollo`
- 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`

Expand Down

Large diffs are not rendered by default.

32 changes: 32 additions & 0 deletions packages/apollo-codegen-scala/src/__tests__/codeGeneration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
17 changes: 14 additions & 3 deletions packages/apollo-codegen-scala/src/codeGeneration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ function dataContainerDeclaration(
insideCompanion
}: {
name: string;
properties: Property[];
properties: (Property & ({ name: string } | { responseName: string }))[];
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
extraSuperClasses?: string[];
description?: string;
insideCompanion?: () => void;
Expand All @@ -460,6 +460,7 @@ function dataContainerDeclaration(
() => {
properties.forEach(p => {
propertyDeclaration(generator, {
jsName: (p as any).name || (p as any).responseName,
propertyName: p.propertyName,
typeName: p.typeName
});
Expand Down Expand Up @@ -489,7 +490,12 @@ function dataContainerDeclaration(
},
() => {
const propertiesIn = properties
.map(p => `${p.propertyName} = ${p.propertyName}`)
.map(
p =>
`"${(p as any).name || (p as any).responseName}" -> ${
p.propertyName
}`
)
.join(", ");
generator.printOnNewline(
`scala.scalajs.js.Dynamic.literal(${propertiesIn}).asInstanceOf[${name}]`
Expand Down Expand Up @@ -536,7 +542,12 @@ function dataContainerDeclaration(
},
() => {
const propertiesIn = properties
.map(p => `${p.propertyName} = ${p.propertyName}`)
.map(
p =>
`"${(p as any).name || (p as any).responseName}" -> ${
p.propertyName
}`
)
.join(", ");
generator.printOnNewline(
`scala.scalajs.js.Dynamic.literal(${propertiesIn}).asInstanceOf[${name}]`
Expand Down
6 changes: 5 additions & 1 deletion packages/apollo-codegen-scala/src/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
111 changes: 43 additions & 68 deletions packages/apollo-codegen-scala/src/naming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) = {
Expand All @@ -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]
}
}
Expand Down