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

Merge conflictive types #642

Closed
francescjaume opened this issue Feb 19, 2018 · 8 comments
Closed

Merge conflictive types #642

francescjaume opened this issue Feb 19, 2018 · 8 comments

Comments

@francescjaume
Copy link

I want to merge two schemas, from remote servers, with conflictive types.

Schema Server 1:

type Query {
  article(id: ID!): Article
}
type Article {
  id: ID!
  title: String
  url: String
}

Schema Server 2:

type Query {
  article(id: ID!): Article
}
type Article {
  id: ID!
  name: String
}

I try to solve this, using this approach:

app.post('/', bodyParser.json(), graphqlExpress({
	schema: mergeSchemas({
		schemas: allSchemas,
		onTypeConflict: (leftType, rightType) => {
			if (leftType instanceof GraphQLObjectType) {
				return mergeObjectTypes(leftType, rightType);
			}
			return leftType;
		}
	})
}));
function mergeObjectTypes(leftType, rightType) {
	if (!rightType) {
		return leftType;
	}
	if (leftType.constructor.name !== rightType.constructor.name) {
		throw new TypeError(`Cannot merge with different base type. this: ${leftType.constructor.name}, other: ${rightType.constructor.name}.`);
	}
	leftType.getFields() // Populate _fields
	leftType.getInterfaces() // Populate _interfaces
	for (const [key, value] of Object.entries(rightType.getFields())) {
		leftType._fields[key] = value;
	}
	for (const [key, value] of Object.entries(rightType.getInterfaces())) {
		leftType._interfaces[key] = value;
	}
	return leftType;
}

But this mergeObjectTypes function only resolves the last apollo-link server.

How can i fix this?

Github complete example: https://github.com/francescjaume/graphql-microservices-example

@4F2E4A2E
Copy link

I can confirm that merging types are working only on Root and Query & Mutation level.
It would be very helpful if second level types (query &/ Mutation) which have no field conflicts could be merged, like :

this type

type Article {
  id: ID!
  name: String
}

and this type

type Article {
  created_at: String 
}

would resolve in this type:

type Article {
  id: ID!
  name: String
  created_at: String 
}

@NicolaiSchmid
Copy link

Hey,
I looked into this problem today and spent a lot of time reading implementations for this problem for example this one.
I tried creating a function of my own. Even though a didn't find any documentation on the AST structure itself of the lefType and rightType I think I may have found a solution that doesn't throw any errors. So I think I might be on the right track.

function mergeObjectTypes(leftType, rightType) {
    if (!rightType) {
        return leftType;
    }

    if (leftType.constructor.name !== rightType.constructor.name) {
        throw new TypeError(`Cannot merge with different base type. This : ${leftType.constructor.name}, other: ${rightType.constructor.name}`);
    }

    if (!leftType.getFields) {
        return leftType;
    }

    // Create new object
    const mergedType = Object.create(leftType);

    // Add name and description of leftType and fallback to rightType
    if (!mergedType.name) mergedType.name = rightType.name;
    if (!mergedType.description) mergedType.description = rightType.description;

    // Populate an array with existing types to compare against
    const mergedFields = new Set();
    mergedType.astNode.fields.map((field) => {
        mergedFields.add(field.name.value);
    });

    rightType.astNode.fields.map((field) => {
        if (!mergedFields.has(field.name.value)) {
            mergedType.astNode.fields.push(field);
        }
    });

    return mergedType;
}

But my implementation actually has no effect towards the final merged schema. The mergedType I'm returning does have the added fields in the astNode property. What might be missing is adjusting the loc property where I don't have any idea what it does or what's its purpose.
Maybe someone reading my comment has additional ideas how to "improve" my script (which actually means making it work 😉).
Thanks!

@manico
Copy link

manico commented Mar 20, 2018

Is there any example of how you can actually merge schemas (left and right type) that works?

@NicolaiSchmid
Copy link

I have yet to find one. That's the reason I tried to build it myself.

@manico
Copy link

manico commented Mar 20, 2018

It seems that merge only works for top level Query.
If you try to merge some type it simply does not work whatever you push in fields.

@NicolaiSchmid
Copy link

Yes, the implementation of graphql-tools only merges the Query, Mutation and Subscription types. Any other type will call the onTypeConflict callback with both types as parameters.

@NicolaiSchmid
Copy link

In my opinion, this is an important issue, since I want to use merging to combine my GraphQL microservice architecture. I think a lot of other users trying to implement similar architectures might benefit from this fix/enhancement as well. The gramps-project from IBM, for example, is built entirely around graphql-toolses merging feature.
It would be nice to see some reactions from the core maintainers of the package about this issue:
@helfer @stubailo @benjamn @freiksenet

@yaacovCR
Copy link
Collaborator

Closed via #1307 rolled into #1306

@yaacovCR yaacovCR mentioned this issue Mar 29, 2020
22 tasks
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

6 participants