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 alias of union types #482

Merged
merged 5 commits into from
Nov 18, 2017
Merged

Fix alias of union types #482

merged 5 commits into from
Nov 18, 2017

Conversation

Jarlakxen
Copy link
Contributor

I was having some issues using union types. I create a test to simulate my conditions.

      it('unions with alias', async () => {
        const mergedResult = await graphql(
          mergedSchema,
          `
            query {
              customerById(id: "c1") {
                ... on Person {
                  name
                }
                v1: vehicle {
                  ... on Bike {
                    bikeType
                  }
                }
                v2: vehicle {
                  ... on Bike {
                    bikeType
                  }
                }
              }
            }
          `,
        );

        expect(mergedResult).to.deep.equal({
          data: {
            customerById: {
              name: 'Exampler Customer',
              v1: { bikeType: 'MOUNTAIN' },
              v2: { bikeType: 'MOUNTAIN' },
            },
          },
        });
      });

This test doesn't pass but is a valid query.

I looked at the code and I found in /src/stitching/mergeSchemas.ts that the method filterSelectionSet is doing this when it find a set of fields of a type:

L612

 if (
        parentType instanceof GraphQLInterfaceType ||
        parentType instanceof GraphQLUnionType
      ) {
        selections = selections.concat({
          kind: Kind.FIELD,
          name: {
            kind: Kind.NAME,
            value: '__typename',
          },
        });
      }

So, is adding '__typename' field to the list of field requested in case of a union type or an interface. But when resolve each of field:

L584

if (
          parentType instanceof GraphQLObjectType ||
          parentType instanceof GraphQLInterfaceType
        ) {
          const fields = parentType.getFields();
          const field =
            node.name.value === '__typename'
              ? TypeNameMetaFieldDef
              : fields[node.name.value];
          if (!field) {
            return null;
          } else {
            typeStack.push(field.type);
          }
        }

Is not considering the case of a union type. I fix this situation by adding the case of the union.

I think this PR could fix #474.

TODO:

  • If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
  • 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
  • Update CHANGELOG.md with your change

@sync
Copy link
Contributor

sync commented Nov 10, 2017

Indeed it does fix #474

@lydell
Copy link

lydell commented Nov 14, 2017

Thanks @Jarlakxen, this saved my day!

@stubailo stubailo merged commit 13ede9c into ardatan:master Nov 18, 2017
stubailo pushed a commit that referenced this pull request Nov 18, 2017
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.

Error running a query on a schema with an union field when schema generated by mergeSchemas
4 participants