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

Fix/transform schema interfaces implementing interfaces #2246

Conversation

m4rw3r
Copy link

@m4rw3r m4rw3r commented Mar 19, 2021

This PR fixes apollographql/federation#227

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

@apollo-cla
Copy link

@m4rw3r: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@m4rw3r m4rw3r marked this pull request as ready for review March 19, 2021 22:02
@noam-c
Copy link

noam-c commented May 28, 2021

Hi folks! I just wanted to see if this PR is going to still be merged -- my team has run into apollographql/federation#227 in our use case and it would be great to have that fixed so we could make use of interface inheritance.

@DanielSinclair
Copy link

DanielSinclair commented Jul 10, 2021

Bumping this issue. This functionality is officially recognized and was merged into the the GraphQL Specification on Jan 10, 2020. See here. This pattern is particularly helpful for reducing downstream implements Node statements for the Connection pagination pattern:

type UserContactsConnection {
  pageInfo: PageInfo!
  edges: [UserContactsEdge]
}

type UserContactsEdge {
  cursor: String!
  node: Contact
}

interface Contact implements Node {
  id: ID!
  type: ContactType!
  name: String
}

enum ContactType {
  FRIEND
  COWORKER
}

type Friend implements Contact {
  id: ID!
  type: ContactType!
  name: String
  bestFriend: Boolean
}

type Friend implements Contact {
  id: ID!
  type: ContactType!
  name: String
  company: Company
}

@TomB15
Copy link

TomB15 commented Sep 17, 2021

Hi, is there any progress on this issue? I would like to support merging this change.

@deodad
Copy link

deodad commented Oct 7, 2021

Apollo team -- anything the community can do to help here? Can re-open PR with the contributor license signed if that would help.

@m4rw3r
Copy link
Author

m4rw3r commented Oct 7, 2021

I did sign the CLA way back, and tests pass locally, the circleci test seems to have failed for some unrelated reason. I guess a rebase could help?

@deodad
Copy link

deodad commented Oct 7, 2021

I did sign the CLA way back, and tests pass locally, the circleci test seems to have failed for some unrelated reason. I guess a rebase could help?

worth a shot!

If we do not rename interfaces we will end up with duplicate
type-instances with identical names and then fail Schema creation.
@m4rw3r m4rw3r force-pushed the fix/transformSchema-interfaces-implementing-interfaces branch from ae29b04 to eb196db Compare October 10, 2021 18:41
@m4rw3r
Copy link
Author

m4rw3r commented Oct 10, 2021

The Windows test is still failing despite rebase, due to the wrong NodeJS version being installed:

#!powershell.exe -ExecutionPolicy Bypass
if ((node --version) -Ne "v${Env:NODE_VERSION}") { exit 1 }


Exited with code exit status 1

CircleCI received exit code 1

The actual used version used above seems to be v14.17.5: https://app.circleci.com/pipelines/github/apollographql/apollo-tooling/12104/workflows/2ed184d5-e5ba-4104-9dea-600b5def5c79/jobs/137143/parallel-runs/0/steps/0-102

But the successful test on master uses v14.16.1: https://app.circleci.com/pipelines/github/apollographql/apollo-tooling/12055/workflows/7b62fcba-3c52-4ee2-8a5d-24a399209fc5/jobs/136772/parallel-runs/0/steps/0-102

@hwillson
Copy link
Member

Thanks for this @m4rw3r - this ended up being resolved in #2456 so I'll close this off.

@hwillson hwillson closed this Oct 27, 2021
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

Successfully merging this pull request may close these issues.

Interfaces implementing interfaces
7 participants