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

'type' is a reserved enum value name #4293

Closed
RoryO opened this issue Jul 26, 2022 · 7 comments
Closed

'type' is a reserved enum value name #4293

RoryO opened this issue Jul 26, 2022 · 7 comments

Comments

@RoryO
Copy link

RoryO commented Jul 26, 2022

Summary

When using a schema that has a type field in an enum, the compiler fails at the task generateServiceApolloSources. The last Apollo Kotlin release I used prior to upgrading 3.2.0, and this definition worked fine with that version.

It looks like there was something similar or identical in #4023, and was possibly fixed in 3.3.1. This seems like a regression to direct someone to using an experimental feature of overriding schema definitions as suggested in the PR and in the error message.

Version

3.4.0

Logs

schema.graphqls: (2180, 3): 'type' is a reserved enum value name, please use the @targetName directive to specify a target name
----------------------------------------------------
[2179]:
[2180]:  """
[2181]:  column name
----------------------------------------------------

Steps to reproduce the behavior

Using this exact schema definition, the compiler errors on the type field

"""
select columns of table "Device"
"""
enum Device_select_column {
  """
  column name
  """
  appBuildNumber

  """
  column name
  """
  brand

  """
  column name
  """
  createdAt

  """
  column name
  """
  id

  """
  column name
  """
  model

  """
  column name
  """
  osVersion

  """
  column name
  """
  personId

  """
  column name
  """
  type

  """
  column name
  """
  uniqueIdentifier

  """
  column name
  """
  updatedAt
}
@martinbonnin
Copy link
Contributor

Hi @RoryO 👋 Thanks for sending this!

The last Apollo Kotlin release I used prior to upgrading 3.2.0, and this definition worked fine with that version.

IIRC, v3 has always used type as a top level property for the CompiledType. This was an unfortunate design decision as __type would have ensured no name clash but it's hard to revert on that one now. We have it on the list of things to do for the next major version.

direct someone to using an experimental feature of overriding schema definitions as suggested in the PR

Indeed. You can use the below

# Inside an extra.graphqls file to put next to your schema.graphqls file

extend enum Device_select_column {
  type @targetName(name: "_type")
}

New directives such as @targetName typically undergo an "experimental" phase but in that specific case, since it's the only solution to your problem, it won't go anywhere.

Hope this helps?

@martinbonnin
Copy link
Contributor

Diving into this again, I realized that since we could display the error message, we could do the escaping automatically so I gave it a shot in #4295.

@BoD that also relaxes a bunch of validation rules. I think it's ok? Let me know what you think

@martinbonnin martinbonnin added the ⌛ Waiting for info More information is required label Jul 27, 2022
@RoryO
Copy link
Author

RoryO commented Jul 27, 2022

Thank you for looking at this. That is good insight of working around the issue internally instead of directing someone to do something undocumented. I will continue to use 3.2 until this is resolved.

@martinbonnin martinbonnin removed the ⌛ Waiting for info More information is required label Jul 27, 2022
@BoD
Copy link
Contributor

BoD commented Jul 28, 2022

@martinbonnin Yeah this looks good to me 👍 It's good to have @targetName but if names can automatically be escaped, it's even better.

@martinbonnin
Copy link
Contributor

Hi @RoryO 3.5.0 now automatically escapes type to type_. Let us know how that goes.

@RoryO
Copy link
Author

RoryO commented Aug 3, 2022

3.5.0 compiles with the same schema without any issues now.

@martinbonnin
Copy link
Contributor

Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants