Adding config options for custom schema type names

[BobaFetters]

-Adding new config option to contain Schema customizations SchemaCustomizations
-Adding property to contain custom schema type names customTypeNames

[BobaFetters]

@AnthonyMDev @calvincestari Wanted to get a draft up of what I was thinking here for the new config option. Thinking its best to have it be more verbose and descriptive, adds a little more to the JSON to do the configuration but makes it very clear what you are doing.

Technically could probably combine the enum for some types into one generic one but keping them separate gives us flexibility going forward to add more options to them individually like the enums or input objects.

Also added a sample of hte config JSON showing what they would look like to decode into this structure (that wont be part of the final commit)

[calvincestari]

It looks like the user is able to specify a bunch of custom type names with this but where do they specify the old schema type that it's being substituted for?

[BobaFetters]

So the CustomSchemaTypeName enum is to give us something better to work with on the swift side vs something like a [String: String] dictionary. So on the Swift side what we end up with in the SchemaCustomizations struct is a [String: CustomSchemaTypeName] dictionary where the String key is the name of the type you are customizing the name for as it exists in the schema, and the enum contains the type of object it is (interface, object, scalar, etc) plus the new name you want, and possibly some other info like changing names of cases on an enum. That way when we are consuming this data on the swift side during code generation we will have the name of a type when we are working with it and we can easily check if there is a customization available for it to use.

[calvincestari]

OK, that's fine. The sample JSON didn't seem to reflect this which was confusing.

Do we anticipate that users will need this degree of flexibility for type renaming? i.e.: one type named many different things in different contexts? I'm mostly opposed to this being a configuration option vs. a directive. For a directive I was thinking if it could be a type extension in the operation document:

extend type User @typeRename("MyNewType")

[BobaFetters]

Well it would still only allow you to rename a type once, not multiple different times since the type you are renaming is used as the dictionary key.

And yea @AnthonyMDev and I discussed config option vs directive and thought this may be the route to go but we can continue that discussions because the data structure I laid out and the further implementation of it will remain the same either way, just how it gets configured would change.

[calvincestari]

Well it would still only allow you to rename a type once, not multiple different times since the type you are renaming is used as the dictionary key.

Yes I see that now, my mistake. I'm really not a fan of the JSON configuration, it's too messy compared to a single directive.

[AnthonyMDev]

I'm more inclined towards the JSON configuration option because it keeps a clear separation between what is client-side vs the actual schema. In your .graphql files, you will still need to use the existing names of the types and enum cases. They will only have the names changed in the generated Swift code. By using a schema extension, it blurs the lines there and makes it less clear.

That said, I think the JSON config format needs to be cleaned up a bit, as this first pass at is is pretty complex and makes actually configuring these things really frustrating and error prone. I'll be posting my opinions on that format in my review shortly.

[calvincestari]

In your .graphql files, you will still need to use the existing names of the types and enum cases.

I agree, having to use the 'old' type name in operations alongside the rename directive is confusing. I think we could probably twist our frontend to allow the 'new' name everywhere but is that more or less desirable.

Configuring this via JSON still doesn't resolve the problem of needing to use old types in operations but new types in Swift. It's still just as confusing and the JSON config usually doesn't form part of the project source so it's a buried change.

[AnthonyMDev]

I think we could probably twist our frontend to allow the 'new' name everywhere but is that more or less desirable.

I think that is less desirable.

Configuring this via JSON still doesn't resolve the problem of needing to use old types in operations but new types in Swift.

I agree with you that it's still a bit confusing having to use the old names in your GraphQL files and the new names in your Swift files, but I think it at least provides a clear line of delineation. You are still writing GraphQL that is valid against your schema. In all places where you are writing GraphQL, you are using the names provided by your schema.

The alternatives would be to either:

• Allow them to use either the new or old name in the graphql
  • This could be even more confusing and adds a lot of complexity
  • We would need to do more transforms of those types to send the old type names in the actual network requests
• Require them to use the new names in the graphql
  • This would also be confusing, as you are only using the new name for types in which you have overridden the names. Types that haven't got a custom name will be the same as they are in your schema.
  • People use their schema and schema docs as a reference when writing their queries. They could also share queries across multiple platforms.

It's still just as confusing and the JSON config usually doesn't form part of the project source so it's a buried change.

This would still be the case with the directives unfortunately. They would just be buried in a schema extension .graphqls file. Because this is changing the names of types for the entire schema, not an individual operation definition, there is no way for this to really exist locally.

I think it's appropriate for this feature to only affect the generated Swift code. And if it only affects the Swift code, I think it's appropriate to keep it as a config option rather than a schema directive. It's never going to be 100% obvious what's going on when you use these custom names. It's not like field aliases that are local to the operation that declares them. These are sweeping overall changes to your schema.

[BobaFetters]

Figured we could name this SchemaCustomizations so it could potentially contain more options in the future.

[AnthonyMDev]

I think the SchemaCustomizations struct and the CustomSchemaTypeName enum provide a good basic structure here. But this is pretty cumbersome to use (both in code and in JSON) and I'd like us to try to add some flexibility here.

---

Swift Config

Let's start with just considering the Swift structs for use with ApolloCodegenLib (ignoring the JSON case).

For the composite types (object, interface, and union) as well as custom scalar, there is currently nothing to change other than the name. While I typically would favor providing all of these cases explicitly to allow for future additions, in this case, it makes things really cumbersome to use and error prone.

You now have to ensure that you are using the right case for each type. If you used the .object case for an interface it would theoretically throw an error, but really this could work just fine and figure out the right thing to do (which is just change the name of the interface type). I don't see any added value in forcing the user to wrap each type in the right case. Combining them into one case would make our user's lives a lot easier without losing any current functionality.

Additionally, when changing enum or input objects, if you only want to change the original name, it would be nice to just pass in the name string without having to worry about getting the right union case.

Functionally, the only time you need to accurately use the correct case here is when you want to change enum case names or input object field names. Even if you were just changing the name of those types, you wouldn't need to indicate the correct type.

This makes me think that we should have one catch-all case for just changing the name. We could call this either .type or .default? Open to other name suggestions.

I actually think it's pretty unlikely that we ever add another field to the cases for object, interface, union, & customScalar, but even if we did, we could easily break them out into additional cases at that time.

---

To make it easier to create these enums, we should make CustomSchemaTypeName conform to ExpressibleByStringLiteral. String literals could always be initialized as the default case.

Then you could just initialize it with:

customTypeNames: [
  "MyObject": "CustomObject,
  "MyInterface": "CustomInterface",
  "MyEnum": "CustomEnum",
  "OtherEnum": .enum(
    name: "CustomOtherEnum,
    cases: [
      "CaseOne": "One",
      "CaseTwo": "Two"
    ]),
  "MyInputObject": "CustomInputObject"
]

Note that MyEnum, and OtherEnum are both enums, but because we aren't changing the case names of MyEnum, we don't need to be specific about the case used to initialize the config.

---

JSON Config

With this structure, the shape of the JSON data can be a lot simpler as well. If we do some custom JSON decoding here, we can remove the need for the nested objects and just infer the enum types based on the fields present. We would need to do a bit of validation (ie. if "cases" exists, then we expect it to be an enum), but it would be a lot easier to write your configuration.

I feel like we should accept both the basic string, or the object JSON form, but we shouldn't require them to indicate the type explicitly.

What I mean is all of this should be a valid JSON object for the "customTypeNames" field:

"customTypeNames": [
  "MyObject": "CustomObject,
  "OtherObject": { "name": "OtherCustomObject" },
  "MyInterface": "CustomInterface",
  "MyEnum": "CustomEnum",
  "OtherEnum": {
    "name": "CustomOtherEnum,
    "cases": [
      "CaseOne": "One",
      "CaseTwo": "Two"
    ]
  },
  "MyInputObject": "CustomInputObject"
]

[AnthonyMDev]

I think we might want to make name an optional String? on this and inputObject. You may want to change the name of cases/fields without changing the name of the type itself.

[BobaFetters]

I agree ease of use is important, but I also think in general some extra complexity for more explicit clarity of what is happening can be useful as well, in this case both from the JSON side and especially having the different enum cases on the swift side in case there may be times that information is needed. I think a setup like that is a nice middle ground between just strings as you suggested, and having a directive which would make it very clear what type was being modified. That said, I'm happy to make the suggested changes and focus on ease of use.

Also yea the name will need to be optional for the scenario you described.

[calvincestari]

.. if it only affects the Swift code, I think it's appropriate to keep it as a config option rather than a schema directive.

I do agree with this statement and the proposed struct/JSON changes are looking good. 👍🏻

Addressed feedback

[AnthonyMDev]

Can you please rebase the base feature/rename-schema-types branch on main. All the additional changes from main merged into this PR are making it difficult to review.

[AnthonyMDev]

This is awesome! Nice job Zach!

[AnthonyMDev]

Nit: would be nice is name came before cases. I know the encoding is alphabetical by default, but we could manually change this. Not that important though.

[BobaFetters]

Would have to test to check but I think in actual use that may be the case since we encode name first in the implementation, but in the Tests specifically we have the encoder setup to do alphabetical order.