Concat document definitions

[evanrs]

Document fragments parsed by graphql-tag/loader lose their definitions when used with a gql tagged template literal. This is fixed by concatenating the document definitions—filtering for duplicates.

Without this, the following would fail:

import catFaceFragment from './catFaceFragment.gql';

const query = gql`
  query Kitten($name: String) {
    kitten(name: $name) {
      ... CatFace
    }
  }
  ${catFaceFragment}
`

#import "./whiskers.gql"

fragment CatFace on Cat {
  ... Whiskers
}

fragment Whiskers on Cat {
  whiskers {
    quantity
    length
    areWiderThanBody
  }  
}

Unknown fragment "Whiskers".

[apollo-cla]

@evanrs: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[jnwng]

@evanrs thank you!

however, i dont fully understand what's breaking here, is this the case where you're mixing usage of interpolation and the #import syntax?

if so, do you mind adding tests here to make sure this works as expected?

[jnwng]

also, processFragments already deals with fragment de-duplication. can you see if we can re-use that or combine the logic so we dont have multiple ways of de-duplicating fragments?

[evanrs]

Will do, thanks for the quick response.

[evanrs]

@jnwng I've added a test that reproduces the problem.

I'm having trouble working this change into the current flow. processFragments relies on the fragmentDefinition having loc.source, but this is absent on definitions accumulated from the interpolated documents.

My attempt quickly cascaded to changes in both parseDocument and processFragments. Can you lend some direction on what you think is best here?

[AmauryLiet]

@evanrs I think merging your work would be highly valuable! Are you still working with graphql-tag on your project(s)?

[evanrs]

Thanks, @amauryliet, I am.

Are you experiencing this issue?

[AmauryLiet]

@evanrs Yes we were doing a refacto by switching from using gql`` tags in .js files to .graphql files and that made some requests fail ;)

Do you think this PR is mergeable or is it too old now?

[jnwng]

Happy to take another look today to see how we can get this in! On Mon, Apr 23, 2018 at 4:57 AM Amaury Liet [email protected] wrote:

@evanrs https://github.com/evanrs Yes we were doing a refacto by switching from using gql`` tags in .js files to .graphql files and that made some requests fail ;)

Do you think this PR is mergeable or is it too old now?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/apollographql/graphql-tag/pull/105#issuecomment-383549743, or mute the thread https://github.com/notifications/unsubscribe-auth/ABERRvea0MY_DtUDyi0xJOmqKuOTzDL6ks5trcHAgaJpZM4OBnot .

[mkochendorfer]

This looks like a great feature improvement. Anything I can do to help move this PR along?

[Vincz]

Hey guys! Any update on this? Can we help one way or another ? @evanrs @jnwng

[evanrs]

@Vincz thanks for reviving this issue 🙂— the test is still not passing on master.

Given the interest in this @jnwng could you provide direction? Or, help us wrangle a member from @apollographql to take the lead on this?

@AmauryLiet, I think it is too old to accept this solution — other than the test case, which remains valid. If you, @Vincz, or @mkochendorfer want an edifying experience into GraphQL's ASTs I can promise creating a new PR on this issue will provide! 😛

[j0nd0n7]

I came to this issue from https://github.com/apollographql/graphql-tag/issues/159 because importing a fragment from a fragment doesn't work anymore. Are these PR solve the issue? if so, could you move forward the PR?

[mzalewski]

Simple solution that seems to have worked for me: https://github.com/mzalewski/graphql-tag/tree/patch-1

Any reason to avoid using the graphql language printer to do this?

[mzalewski]

Converted my code into a pull request (#227) - it's probably slightly inefficient since it's parsing, printing then parsing again - but it works for now and should fix the issue until someone figures out a better solution