Code generation incorrect for field 'self'

[lilyball]

If I have a field named self in a GraphQL query, the code generator produces code that references Self.selections, which is invalid as Self is a keyword and so the code fails to compile. The current workaround is to alias the field to a different name, though unfortunately the name self_ doesn't work as Apollo still produces the type name Self.

The code generator should recognize keywords and work around them somehow, either by judicious insertion of backticks in the generated code, or by renaming the type.

[JustinDSN]

Hi @kballard I've tried to reproduce this issue with the latest version of Apollo-Codegen (apollographql/apollo-tooling@7723fcd).

I updated an existing test to have a property declaration with the name of 'self' and the generator correctly escaped it.

The Test File was found here: /apollo-codegen/test/swift/language.ts

it(`should generate an escaped struct declaration`, () => {
    generator.structDeclaration({ structName: 'Type' }, () => {
      generator.propertyDeclaration({ propertyName: 'name', typeName: 'String' });
      generator.propertyDeclaration({ propertyName: 'yearOfBirth', typeName: 'Int' });
      generator.propertyDeclaration({ propertyName: 'self', typeName: 'String' });
    });

    expect(generator.output).toBe(stripIndent`
      public struct \`Type\` {
        public var name: String
        public var yearOfBirth: Int
        public var \`self\`: String
      }
    `);
  });

I also verified that both 'self', and 'Self' are in the reserved keywords list defined here: https://github.com/apollographql/apollo-codegen/blob/master/src/swift/language.ts#L35-L53

Can you confirm if this is still an issue and better steps to reproduce?

[designatednerd]

@lilyball Is this issue still occurring? The reserved keywords list has moved to here: https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-codegen-swift/src/language.ts#L41 and it does still contain both self and Self as reserved keywords.

[lilyball]

Yes it's still occurring. I just added a self field to a query and regenerated using Apollo 2.15.0 and while it escaped the tokens in most places, it didn't do it in all places. In particular, in the diff I see

public var `self`: Self? {

and the usage of the unescaped Self here is wrong.

[designatednerd]

Innnnteresting. I'll have a look! Thanks for confirming.

[designatednerd]

@lilyball OK finally gathering the courage to dive into typescript and deal with this. So I can understand better what's happening here, is Self a type in your GraphQL schema?

[lilyball]

No, but self is a field, and each field in the query is given a nested type named after it in the generated Swift code.

[lilyball]

Which is to say, if I use selfView: self { … } instead of self { … } then the generated code declares public var selfView: SelfView? { … } instead.

[designatednerd]

OK - that will help me at least come up with some kind of test for it, at least. Thank you!

[designatednerd]

Fix shipped with 0.14.0!

[lilyball]

I finally got around to testing this and apollo version 2.18.0 is still broken. I see escaping in most places, except for optionals:

public var `self`: Self? {
    …
}

I think the problem is the type is Self? instead of Self. escapeIdentifierIfNeeded seems to be doing just bare string matching. We should probably have escapeTypeNameIfNeeded that handles types. Not just Self? but I'd imagine we'd have to handle [Self] and even [Self?]?. A simple way to handle this that would be fairly generic would be to just identify all valid identifier substrings and escape those independently.

[designatednerd]

Ergh, I agree - I'm going to add a task for this to the iOS Swift Codegen project so I don't forget about it.

@lilyball If you're interested in making a fix on the TypeScript stuff in the meantime, we'd love to see a PR on the tooling repo