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

Incorrect Type Conversion for Lists with Shared-model type #898

Closed
cavega opened this issue Apr 30, 2018 · 5 comments
Closed

Incorrect Type Conversion for Lists with Shared-model type #898

cavega opened this issue Apr 30, 2018 · 5 comments

Comments

@cavega
Copy link

cavega commented Apr 30, 2018

We have a subquery photos where 2 fields (or subqueries) called published and archived that share the same properties or subqueries:

photos {
    published {
        id
       thumbnail
       etc....
    }

    archived {
        id
        thumbnail
        etc....
    }
}

The schema.json generated by Apollo Codegen is shown below. As you can see, both published and archived are objects of type LIST with items of type VenuePhoto:

[
{
          "kind": "OBJECT",
          "name": "Photos",
          "description": "",
          "fields": [       
            {
              "name": "published",
              "description": "the published photos",
              "args": [],
              "type": {
                "kind": "LIST",
                "name": null,
                "ofType": {
                  "kind": "OBJECT",
                  "name": "VenuePhoto",
                  "ofType": null
                }
              },
              "isDeprecated": false,
              "deprecationReason": null
            },
            {
              "name": "archived",
              "description": "the archived photos",
              "args": [],
              "type": {
                "kind": "LIST",
                "name": null,
                "ofType": {
                  "kind": "OBJECT",
                  "name": "VenuePhoto",
                  "ofType": null
                }
              },
              "isDeprecated": false,
              "deprecationReason": null
            }            
          ]
        },
{
          "kind": "OBJECT",
          "name": "VenuePhoto",
          "description": "",
          "fields": [
            {
              "name": "id",
              "description": "the unique ID of this photo",
              "args": [],
              "type": {
                "kind": "SCALAR",
                "name": "String",
                "ofType": null
              },
              "isDeprecated": false,
              "deprecationReason": null
            },
            {
              "name": "thumbnail",
              "description": "a thumbnail image for this photo",
              "args": [
                {
                  "name": "treatment",
                  "description": "",
                  "type": {
                    "kind": "SCALAR",
                    "name": "String",
                    "ofType": null
                  },
                  "defaultValue": null
                }
              ],
              "type": {
                "kind": "OBJECT",
                "name": "Image",
                "ofType": null
              },
              "isDeprecated": false,
              "deprecationReason": null
            },
            {
              "name": "fullSize",
              "description": "a full-size image for this photo",
              "args": [
                {
                  "name": "treatment",
                  "description": "",
                  "type": {
                    "kind": "SCALAR",
                    "name": "String",
                    "ofType": null
                  },
                  "defaultValue": null
                }
              ],
              "type": {
                "kind": "OBJECT",
                "name": "Image",
                "ofType": null
              },
              "isDeprecated": false,
              "deprecationReason": null
            }                   
          ]
        }
]

The current issue we are facing with Apollo Android is that the generated models are of the following type;

List<Archived> archived;
List<Published> published;

where we are expecting the following:

List<VenuePhoto> archived;
List<VenuePhoto> published;

A failed attempt to coerce the tool to generate the right models was to extra everything inside photos{} as a fragment but that did not change the results.

I would appreciate some feedback as to whether the current behavior and model output is either a bug or if it is working as intended and there is a missing feature or flag that I need to incorporate in my *.graphql file to achieve the desired results.

@ghost ghost added the blocking label Apr 30, 2018
@sav007
Copy link
Contributor

sav007 commented May 1, 2018

If you want them to share the same model you have to use fragment:

photos {
    published {
       ...VenuePhotoFragment
    }

    archived {
        ...VenuePhotoFragment
    }
}

fragment VenuePhotoFragment on VenuePhoto {
   id
   thumbnail
   etc....
}

It will generate Archived.Fragments and Published.Fragments classes that have the same reference to VenuePhotoFragment. Yes it still generates 2 different models for Archived and Published because the set of requested fields might be different as well as arguments for requested fields.

At the same time I agree that we should recognize identical sub-queries and have generated only one model class instead of 2 identical ones.

@cavega
Copy link
Author

cavega commented May 1, 2018

@sav007 That's exactly what I ended up doing but, again, because it is still 2 different models my RecyclerView code looks kinda ugly. I will proceed with the suggestion for the time being. However, is there any chance that a future version of Apollo-Android could enhance/fix this so that a single model class is used? Thanks for the feedback.

@sav007
Copy link
Contributor

sav007 commented May 4, 2018

We encourage you to use ViewModels for UI, that layer of abstraction will give you flexibility and resolve such issues, as in many cases generated models don't satisfies UI needs and requires to have additional fields or different types to present on UI

@cavega
Copy link
Author

cavega commented May 4, 2018

That's true but there really shouldn't be a reason for the generated code to break the contract defined in the schema.json file (i.e. the 2 lists use the same data type). Thanks for the feedback. I'll revisit my solution.

@digitalbuddha
Copy link
Contributor

Closing for now. Let us know if any recommendations of how to make experience better.

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

No branches or pull requests

3 participants