amplify-swift icon indicating copy to clipboard operation
amplify-swift copied to clipboard

Add CaseIterable to enums, or allow codegen files to be locked from being updated

Open LordZardeck opened this issue 5 years ago • 5 comments

Describe the solution you'd like Currently I can't modify the Enum generated from codegen, because when models are built again, it overrides my files. I feel like I should be able to lock down a single file from being modified anymore so that I can make modifications. My current use case is that I'd like to add the CaseIterable protocol to the enums, but I can't as my modifications get override during codegen.

LordZardeck avatar Jun 29 '20 18:06 LordZardeck

It's not clear that either of these is the right solution to this problem.

Automatic CaseIterable conformance

Automatically declaring CaseIterable works as long as the schema author always preserves the order of the enum in the source GraphQL schema. If a subsequent edit changes the schema, it would change the generated iteration order. While that's admittedly a danger even in the world of native Swift, at least there you're usually dealing with the Swift source directly, which removes a layer of indirection and makes it more likely to be alert to those kinds of changes. But I'm reluctant to do this kind of automatic conformance without a fairly compelling benefit, and right now CaseIterable can be manually adopted with a relatively low-cost workaround:

Foo.swift (code generated)

enum Foo {
    case foo
    case bar
    case baz
}

Foo+CaseIterable.swift (manually maintained)

extension Foo: CaseIterable {
    static var allCases: [Foo] {
        [.foo, .bar, .baz]
    }
}

Locking code generated files

My initial thought is that this one is a non-starter. A model's single source of truth is the GraphQL schema, that Amplify uses to generate language-specific model and schema files and to generate the GraphQL backend. The internals of the code-generated model & schema files are largely opaque to the host app--in fact, in 1.0.2 we just went through an effort to doc-comment those files with a warning to that effect. Locking a code generated file means that subsequent upgrades of the Amplify codegen infrastructure would not apply their changes to that locked files, potentially resulting in hard-to-diagnose bugs.

Alternative: Specify language overrides

Perhaps a more general solution would be to extend our GraphQL schema with directives to allow language-specific overrides to be sprinkled throughout the schema file. I could see something like:

schema.graphql (hypothetical, doesn't actually work)

enum Foo
@swift(
  protocols: ["CaseIterable", "Hashable", "Equatable"]
) {
  foo
  bar
  baz
}

type MyType @model {
  id: ID!
  fooType: Foo!
}

That maintains the single source of truth in the GraphQL schema, while putting the power in the hands of the developer to not only adopt the protocol, but be responsible for schema changes that may change how that protocol is evaluated in their code.

This isn't a slam dunk: We'd have to figure out how this works on other generated runtimes as well, as it's not immediately apparent whether Java or JS would benefit from this. I'm also wondering if this is useful outside compiler-synthesized conformances, since (I think) non-synthesized conformances are declarable in extensions housed in separate files.

We'll leave this open as a feature request (I think it's more than an enhancement at this point) to see if it gathers any interest over the next few weeks. You might consider asking folks on the Discord channel to chime in on this feature request to see if there's any community interest in such a feature.

Of course, other proposals to solve this are quite welcome. :)


NEXT STEPS:

  • Leave this open until 31-Jul-2020 to see if there is any community desire for this feature. If not, close. If so, retain as a feature request until we can prioritize it onto the backlog.

palpatim avatar Jun 29 '20 22:06 palpatim

The proposed solution for language specific directives/overrides seems like a great compromise.

Our use case is similar (swift), we add Equatable to a number of our structs, but when running codegen this gets overridden.

seanvm avatar Mar 26 '21 17:03 seanvm

I personally would like to see the models move in the opposite direction. The generated models should be good at one thing only and that thing is being DTOs between our apps and the AWS SDK. The more complicated they get and the more features you start building into them the more likely AWS will get it wrong. A proper app design will not use these DTO objects anywhere in the app other than when directly working with the Amplify SDK. You should be isolating the SDKs from the remainder of your app. Not inviting them in. Do yourself a favor and build a DTO to/from your Domain layer just above the AWS SDKs.

kjones avatar Mar 26 '21 21:03 kjones

Linking related issue for reference https://github.com/aws-amplify/amplify-ios/issues/1890 This issue also briefly discusses a few options and includes a simple script that can be run after codegen'ing models to add CaseIterable conformance to all enums. Note that this works with how models are generated today, but may not necessarily always work if that changes down the road. While still not ideal, it can help make adding these conformances easier.

atierian avatar Jul 26 '22 13:07 atierian

Linking another related issue https://github.com/aws-amplify/amplify-swift/issues/521

lawmicha avatar Aug 10 '23 15:08 lawmicha