Experimental Subschema empty interface

[tomgasson]

In order to work out what part of our very large schema is actually being used by our products, we've been exploring using the subschema extraction. Our intent is to identify dead / unused schema and make our breaking changes detection & test selection more fine-grained (specific to the consuming product)

We've run into a problem of producing invalid schema.

schema

type Query {
  search: Searchable
}

interface Searchable {
  title: String
  score: Float
}

type Article implements Searchable {
  title: String
  score: Float
  body: String
}

usage

graphql`
  query SearchQuery {
    search {
      ... on Article {
        body
      }
    }
  }
`;

output

type Article implements Searchable {
  body: String
}

type Query {
  search: Searchable
}

interface Searchable

which is invalid, since Searchable has no fields.

1. We can't remove Searchable because it appears as an output type of Query.search: Searchable and changing that would change the schema
2. We can't "fake" it with a dummy field because the Article would then need to implement that dummy field too
3. We can't use __typename (which is effectively what happens at runtime) because __ is reserved

This PR implements a "fix": If we find empty interfaces, we force their fields existence (and hence their implementing concrete types to have those fields). This means we do have "unused fields", but only for these cases.

Not ideal, however this only occurs 12 times for us so is acceptable, and I expect it's not encountered in Meta's schema if it's not yet addressed.

[mjmahone]

Ah thank you for helping with this! However, this specific PR is flipping the purpose of SchemaSet on its head. SchemaSet is meant to produce intermediate, spec-invalid subsets, which we can combine/merge and individually evaluate. Only the merge of ALL subsets must produce a valid full schema (and even whether your exposed schema should be spec-valid is IMO debatable).

We do already have some "fixing" capabilities in fix_all_types: https://github.com/facebook/relay/blob/main/compiler/crates/schema-set/src/schema_set.rs#L290

But I didn't put a ton of care into that, and we almost certainly could use a pulling-apart of what exactly we are fixing and breaking down each sub-fixing component then re-composing them into a specific "fix for this purpose" pipeline.

So there's kind of three possible paths:

• you could update your tooling to break the spec and allow Searchable to be empty. This is what we're doing, and why we don't worry about empty interfaces.
• you could "clean up" empty interfaces: remove the empty interfaces from the schema set, and remove all references to them. We have something like this here: https://github.com/facebook/relay/blob/main/compiler/crates/schema-set/src/set_remove_defined_references.rs#L49. This though can lead to problems where types are referenced in executable docs that are now gone from the schema.
• Alternatively we should have a pass that translates empty interfaces into unions, if you want to make sure the SchemaSet produced outputs into a valid, spec-compliant SDL without breaking existing executable documents. This can be problematic if your interface implements another interface, but... in that case your interface shouldn't be empty unless both are empty, in which case both could be independent, overlapping unions!

[tomgasson]

update your tooling to break the spec and allow Searchable to be empty. This is what we're doing, and why we don't worry about empty interfaces

I think I'm convinced to do the same. Thanks for the response and detail