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

Codegen produces compile error - Value type 'PhoneCall' cannot have a stored property that recursively contains it

Open anthonymoretti opened this issue 4 years ago • 6 comments

Describe the bug Codegen produces model files with the following compile errors:

  • Value type 'PhoneCall' cannot have a stored property that recursively contains it
  • Value type 'Transcript' cannot have a stored property that recursively contains it

To Reproduce

  1. Create the schema below
  2. Add api, push, then run amplify codegen models
  3. Observe compile error in Xcode

Expected behavior Generated code compiles.

Environment:

  • Amplify Framework Version: 4.21.0
  • Dependency Manager: Cocoapods
  • Swift Version : 5.0

Device Information:

  • Device: Simulator
  • iOS Version: iOS 13.5

Additional context Schema:

type Person @model {
  id: ID!
  name: String!
  callerOf: [PhoneCall!] @connection(name: "PhoneCallCaller")
  calleeOf: [PhoneCall!] @connection(name: "PhoneCallCallee")
}

type PhoneCall @model {
  id: ID!
  caller: Person! @connection(name: "PhoneCallCaller")
  callee: Person! @connection(name: "PhoneCallCallee")
  transcript: Transcript @connection(name: "PhoneCallTranscript")
}

type Transcript @model {
  id: ID!
  text: String!
  language: String
  phoneCall: PhoneCall! @connection(name: "PhoneCallTranscript")
}

anthonymoretti avatar Jun 01 '20 16:06 anthonymoretti

I am also having this issue. It seems that it doesn't support more complex schemas with the models referring back to one another? I feel like that should be resolved or mentioned in their documentation at least.

alexmay48 avatar Jun 05 '20 06:06 alexmay48

@AMAYUT I've been told it might have to do with the use of struct vs class

anthonymoretti avatar Jun 05 '20 20:06 anthonymoretti

Some context here: I started using AWS Amplify over a year ago for my senior capstone project for my computer science degree. We stopped development for about 5 months after we graduated in December, 2019. So, the documentation and schema we made is old (relative to Amplify and how fast they update things). Back then, they had the @connection directive on both ends of the relationship, like you have above @anthonymoretti.

@anthonymoretti You are right that the issue is that structs can't have recursive structures, and that is what is generated when using the old way of having the @connection directive on both ends of the relationship.

To satisfy this, and other things related to data store, it looks like they changed their documentation significantly along with changing how they use the @connection directive. They now combine the @key and @connection directives for one to many (and many to many) relationships. See their documentation here: https://docs.amplify.aws/cli/graphql-transformer/directives#has-many

I did find this link here for some direction on changing to combining the @key and @connection directives: https://github.com/aws-amplify/amplify-cli/issues/1656 However, it still isn't clear, and I haven't found a good guide for changing to use this new way.

This is my biggest complaint about Amplify; they make changes so fast, and they don't give documentation on how to update from the old version. Making changes to your schema is complicated and costly, where using SQL schema is a lot more straight forward to me. This is an especially big change where the original users were probably all using the @connection directive, and so I don't know if I am searching right, but I feel like a lot of amplify users should be struggling to adapt to use data store.

Regardless, we are lucky enough that we don't have our application out into production, and amplify is quick enough development that we can get things up quickly under this new schema change. However, if they make a big enough change like this in the future while we are in production, this would be a big pain. I will try and find a better guide on how to change the schema to utilize data store and remove the old way of using the @connection directive, and without having to start from scratch.

alexmay48 avatar Jun 18 '20 15:06 alexmay48

Thanks @AMAYUT, and yeah I'm using @connection with keyName now. I've made a feature request for a different @connection directive that doesn't use keys and doesn't require naming of the connection actually RFC - Simplified @connection directive aws-amplify/amplify-category-api#305

anthonymoretti avatar Jun 18 '20 21:06 anthonymoretti

@anthonymoretti @alexmay48 thanks for taking the time to comment. We’re working on ways to make DataStore relationships simpler. We’ll update this issue when we have more information.

diegocstn avatar Jun 18 '21 16:06 diegocstn

Related to https://github.com/aws-amplify/amplify-ios/issues/505

chrisbonifacio avatar Sep 09 '22 19:09 chrisbonifacio

With the launch of TransformerV2 directives (hasOne/belongsTo/hasMany/etc), I started off by translating the schema over to

type PhoneCall @model {
  id: ID!
  callerId: ID! @index(name: "byCaller")
  calleeId: ID! @index(name: "byCallee")
  caller: Person! @belongsTo(fields: ["callerId"])
  callee: Person! @belongsTo(fields: ["calleeId"])
  transcript: Transcript @hasOne
}

type Person @model {
  id: ID!
  name: String!
  callerOf: [PhoneCall!] @hasMany(indexName: "byCaller")
  calleeOf: [PhoneCall!] @hasMany(indexName: "byCallee")
}

type Transcript @model {
  id: ID!
  text: String!
  language: String
  phoneCall: PhoneCall @belongsTo
}

Currently, codegen will create Struct types where PhoneCall will have a reference to Transcript, and vice versa, causing the Swift compile issue of "Value type 'PhoneCall' cannot have a stored property that recursively contains it". We are currently working on updating the codegen https://github.com/aws-amplify/amplify-codegen/pull/504 to generate a wrapper type to hold the connected model, ie.

public struct PhoneCall: Model {
  internal var _transcript: LazyReference<Transcript>
  public var transcript: Transcript?   {
      get async throws {
        try await _transcript.get()
      }
    }
public struct Transcript: Model {
  internal var _phoneCall: LazyReference<PhoneCall>
  public var phoneCall: PhoneCall?   {
      get async throws {
        try await _phoneCall.get()
      }
    }

This allows response payloads to be decoded successfully into these types. The new codegen work will be behind a feature flag and released with a dependency on the library changes in https://github.com/aws-amplify/amplify-swift/pull/2583 . I've been using the above schema in the library integration tests to ensure that the CRUD operations work as expected (https://github.com/aws-amplify/amplify-swift/pull/2583/commits/4d51c1bddf5264253e2f24f983216eebdf07b7de) . Will provide an update here once the PRs get merged and released.

lawmicha avatar Dec 31 '22 00:12 lawmicha

The Lazy Loading feature was released in version 2.4.0. This should address this issue.

This support requires the lazy loading feature flag. Please use the latest version of the amplify CLI and review or make updates to cli.json (in the amplify folder) to ensure you have the following settings:

"transformerversion":2 "respectprimarykeyattributesonconnectionfield": true "generatemodelsforlazyloadandcustomselectionset": true

Please open a new issue if this release does not resolve the problem.

ameter avatar Feb 17 '23 17:02 ameter