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

API should support adding types to an already existing schema #3440

Closed
Cito opened this issue Dec 23, 2021 · 9 comments
Closed

API should support adding types to an already existing schema #3440

Cito opened this issue Dec 23, 2021 · 9 comments

Comments

@Cito
Copy link
Member

Cito commented Dec 23, 2021

A user of the Python port of GraphQL.js recently came up with the following question, which I think should be better discussed here:

They are given a schema object that contains an interface (built using SDL). Now they want to programmatically (dynamically) add concrete types implementing this interface to the schema. How would they do that?

Of course, you could instead add the concrete types to the SDL and build the schema from that, or you could instead build a query type and the interface type programmatically, add the concrete types, and then build the schema from the query type. But it would be nice if you could also use the mixed approach, creating the schema first, and then adding the types.

However, adding the concrete types to an existing schema is tricky, because you need to do three things:

  1. Add the type to the _type_map.
    • Ok, this is simple, since you can access the _type_map with getTypeMap().
  2. Add the type to the _implementations_map for the interface.
    • This is is a problem because you cannot access the private _implementations_map or add a new key to this map. We have the getImplementations() method, but if the interface is not yet implemented in the given schema, the returned object cannot be used for updates, and there is no setImplementations() method.
  3. If the schema was already validated, reset the _sub_type_map cache.
    • This is also problematic because _sub_type_map is private and there is no official way to reset it.

So the question is:

  • Should we provide an add_type() method (caring for all of the above) or alternatively, a setImplementations() and some kind of reset() method that would reset the _sub_type_map, so that you can subsequently add types to an already existing schema?

To illustrate the problem, here is the code example from the original issue translated from Python to JavaScript:

// Create a schema with an interface via SDL

const sdl = `
enum Episode { NEWHOPE, EMPIRE, JEDI }

interface Character {
  id: String!
  name: String
  friends: [Character]
  appearsIn: [Episode]
}

type Query {
  hero(episode: Episode): Character
}
`;

const schema = buildSchema(sdl);

// Add concrete types programmatically

const CHARACTERS = ["Human", "Droid", "Animal", "Fungus", "Alien"];

const characterInterface = schema.getType("Character");
const episodeClass = schema.getType("Episode");
const query = schema.getType("Query");

const const typeMap = schema.getTypeMap();
const implementations = { objects: [], interfaces: [] };
schema._implementationsMap[characterInterface.name] = implementations;

for (const character of CHARACTERS) {
  const concreteCharacter = new GraphQLObjectType({
    name: character,
    interfaces: [characterInterface],
    fields: {
      id: { type: new GraphQLNonNull(GraphQLString) },
      name: { type: GraphQLString },
      friends: { type: new GraphQLList(characterInterface) },
      appearsIn: { type: new GraphQLList(episodeClass) },
      primaryFunction: { type: GraphQLString },
    },
  });
  typeMap[character] = concreteCharacter;
  implementations.objects.push(concreteCharacter);
  const name = character.toLowerCase();
  query._fields[name] = {
    name,
    type: concreteCharacter,
    args: [{ name: "id", type: new GraphQLNonNull(GraphQLString) }],
  };
}

schema._subTypeMap = {};  // need to reset this because we added types

console.log(printSchema(schema));

The code above should print this result.

@yaacovCR
Copy link
Contributor

The existing way to do that would seemingly be to use the toConfig methods to recreate a given schema programmatically, modified as necessary. Within graphql tools, we have a mapSchema method that provides an easier framework for that, basically a superset of @IvanGoncharov lexicographicallySortSchema .

i think dynamic schemas are very interesting, but I think in terms of the direction so far from the library, it seems like the idea has been to freeze schemas after creation.

maybe it’s to ensure validation can be taken for granted, although of course that flag could be reset as you mention.

would love to hear @IvanGoncharov thoughts!

@yaacovCR
Copy link
Contributor

@yaacovCR
Copy link
Contributor

If I understand correctly the example, however you don’t need mapSchema, you can just use schema.toConfig and recreate the schema with the addition of the new types. Adding concrete types to a Union would require Union type mapping, because unions hold references to their implementation while object and interface types hold references to the interfaces they implement

@Cito
Copy link
Member Author

Cito commented Dec 23, 2021

@yaacovCR thanks for the feedback. Right, recreating the schema from a modified config would be another option, and probably less error-prone (if you don't care about things like those you mentioned). This would work similarly in Python. Unfortunately, we don't have something like graphql-tools in Python land.

Here is the modified code using this solution:

// Create a schema with an interface via SDL

const sdl = `
enum Episode { NEWHOPE, EMPIRE, JEDI }

interface Character {
  id: String!
  name: String
  friends: [Character]
  appearsIn: [Episode]
}

type Query {
  hero(episode: Episode): Character
}
`;

let schema = buildSchema(sdl);

// Add concrete types programmatically

const CHARACTERS = ["Human", "Droid", "Animal", "Fungus", "Alien"];

// Recreate query with added fields

let query = schema.getQueryType();
const characterInterface = schema.getType("Character");
const episodeEnum = schema.getType("Episode");

const queryConfig = query.toConfig();
const fields = queryConfig.fields;
for (const character of CHARACTERS) {
  const concreteCharacter = new GraphQLObjectType({
    name: character,
    interfaces: [characterInterface],
    fields: {
      id: { type: new GraphQLNonNull(GraphQLString) },
      name: { type: GraphQLString },
      friends: { type: new GraphQLList(characterInterface) },
      appearsIn: { type: new GraphQLList(episodeEnum) },
      primaryFunction: { type: GraphQLString },
    },
  });
  const name = character.toLowerCase();
  const field = {
    type: concreteCharacter,
    args: { id: { type: new GraphQLNonNull(GraphQLString) } },
  };
  fields[name] = field;
}

query = new GraphQLObjectType(queryConfig);

// Recreate schema with modified query (including added types)

schema = new GraphQLSchema({...schema.toConfig(), query, types: []});

console.log(printSchema(schema));

@Cito
Copy link
Member Author

Cito commented Dec 23, 2021

But would like to get @IvanGoncharov's opinion as well. Shall we consider schemas frozen or shall we consider adding a method to update an existing schema by adding more types?

@yaacovCR
Copy link
Contributor

Allowing such methods could allow resolvers to potentially call them, as resolvers are provided schema within info argument.

This could allow alteration of schemas in the midst of execution, which could lead to potentially interesting, non deterministic, and/or undesired results.

One argument would be user-beware, just stick a note somewhere that users of this library should not do that. But it is a relevant wrinkle. Would also love to hear from @IvanGoncharov

@IvanGoncharov
Copy link
Member

@Cito People do crazy things with GraphQL Schema, e.g. they create per-user (or per-role) schemas by applying certain transformations on the common schema.
Having a mutable schema means you can unintentionally affect all copies of that schema.

An even simpler example would be caching of schemas (e.g. caching version of buildClientSchema) so if we change schema afterward it will poison the cache.
So I would say the "immutability" requirement goes even beyond execution (as in @yaacovCR example) but apply to the whole life-cycle.

As discussed in #3226 we need a transformation function in the core especially since we have extendSchema and lexicographicallySortSchema that would use this function internally.

@Cito
Copy link
Member Author

Cito commented Dec 26, 2021

@IvanGoncharov Agree, this sounds like the cleanest solution.

Maybe it should just be documented that schemas are considered immutable.

@Cito Cito closed this as completed Dec 26, 2021
@alexchamberlain
Copy link

Thanks everyone for the useful discussion - the fact that schemas shouldn't be changed during execution really resonated with me.

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

4 participants