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

Allow interfaces to have no implementors #1376

Merged
merged 2 commits into from
Jun 8, 2018
Merged

Conversation

mjmahone
Copy link
Contributor

@mjmahone mjmahone commented Jun 8, 2018

Reverts #1277, and implements spec removal from graphql/graphql-spec#459 (which reverts spec change graphql/graphql-spec#424). While it's an open question whether this validation is good, we need to provide an upgrade path for schemas that currently do not satisfy this validation rule.

Reverts #1277, and implements spec removal from #459. While it's an open question whether this validation is good, we need to provide an upgrade path for schemas that currently do not satisfy this validation rule.
@IvanGoncharov IvanGoncharov added the spec RFC Implementation of a proposed change to the GraphQL specification label Jun 8, 2018
@mjmahone
Copy link
Contributor Author

mjmahone commented Jun 8, 2018

As a heads up, I'm going to merge this commit (even without the spec merge) and cut another rc, because this will make it possible for Facebook to upgrade (which means all Facebook's third-party libraries, like Relay, can upgrade too). I'm very open to figuring out some way to optionally include this schema validation, and provide an opt-in mechanism.

@mjmahone mjmahone merged commit 9925e50 into master Jun 8, 2018
@mjmahone mjmahone deleted the interface-no-implementors branch June 8, 2018 20:44
@IvanGoncharov
Copy link
Member

@mjmahone I'm still forming my position on spec changes. So I'm interested in exploring alternatives.
Switching from a type to interfaces shouldn't affect client queries.

Can you please check if converting non-implemented interface to a type changes Flow typings?
Like converting this:

interface Foo {
  foo: String
}

To this:

type Foo {
  foo: String
}

@IvanGoncharov
Copy link
Member

cut another rc,

@mjmahone Can you please set correct npm tag:
#1375 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed spec RFC Implementation of a proposed change to the GraphQL specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants