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

Node object from interface implementation results in duplicate ID #172

Open
woylie opened this issue Nov 30, 2020 · 2 comments
Open

Node object from interface implementation results in duplicate ID #172

woylie opened this issue Nov 30, 2020 · 2 comments

Comments

@woylie
Copy link

woylie commented Nov 30, 2020

I defined an interface like this:

  interface(:notification) do
    field :id, non_null(:id)
    # a bunch of other fields with resolver functions that resolve the same for every interface implementation
  end

And there's a connection:

  connection(node_type: :notification)

  node object(:me) do
    connection field :notifications, node_type: :notification do
      resolve &Notification.list_my_notifications/2
    end
  end

And then I'm defining the concrete implementation as a node object like this. And since I don't want to copy all the fields including the resolver functions to every interface implementation, I'm using import_fields to import them from the interface.

  node object(:notification_about_something_specific) do
    interface :notification
    import_fields :notification

    # plus specific fields for this notification type
  end

And this basically works as expected when using the API. However, there is one problem: By using both node and import_fields, two :id field definitions are created. And those two ID fields will end up in the SDL file generated with mix absinthe.schema.sdl, which then causes tools that read that file to return an error.

I'm not sure how to fix this. I can move all the field definitions to the interface implementations instead of using import_fields, but that kind of defeats the purpose of defining an interface. And I can remove the id field from the interface definition, so that only the implementations define it, but that seems logically wrong.

I wonder if it would make sense if the node macro overwrites any existing id field instead of just adding one?

@dolfinus
Copy link
Contributor

dolfinus commented Dec 3, 2020

I've got the same error without explicit :id field in the interface, just connection node + node object with import_fields is enough:

interface :revision do
    field :version, non_null(:integer)
    field :created_at, non_null(:datetime)
end

connection node_type: :revision

node object :user_revision do
    interface :revision
    import_fields :revision
    
    field :role, non_null(:user_role)
end

Resulting SDL looks like:

interface Revision {
  version: Int!
  createdAt: DateTime!
}

interface Node {
  "The id of the object."
  id: ID!
}

type UserRevision implements Revision & Node {
  "The id of the object."
  id: ID!

  "The ID of an object"
  id: ID!

  version: Int!

  createdAt: DateTime!

  role: UserRole!
}

type UserRevisionEdge {
  cursor: String
  node: UserRevision
}

type UserRevisionConnection {
  pageInfo: PageInfo!
  edges: [UserRevisionEdge]
}

@benwilson512
Copy link
Contributor

Honestly, I didn't realize people were using import_fields on interfaces. I can see how it is sometimes convenient but the moment you need a custom resolver things can get a bit weird. We can work to make import_fields deduplicate, but there are trade offs. The duplicate ID thing without an explicit :id field is weird, will look into it.

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