Fragments used in Fragments are leading to big source.body string

[marvinsemmelroth]

We have a really big query in our application which is orchestrated by many (mostly the same) fragments. When parsing to a DocumentNode, the parser inlines all does fragments (Without checking for duplicates) into one big string (2-2.5MB - 180k Lines). Some fragments are up to 1.600 times references, which is taking up alot of memory in the JS Heap.

I think a solution would be to move all Subfragments to the query declaration but since we also use these fragments to access the cache (using apollo-client), we still need those fragments to be working standalone.

The fragments are already parsed DocumentNodes so also parsing them again and again should be pretty imperformant. My suggestion would be to add the possibility to pass already parsed definitions to the parser/to the Source-Construct, which then can be used instead of creating them again.

[JoviDeCroock]

Made a quick reproduction as it's easier to talk about these things that way, the reason it's getting inlined is probably more related to the wrapper around parse that you are using than parse itself. That being said, parse allows you to pass {noLocation:true} which will prevent loc from existing.

Looking at the following https://stackblitz.com/edit/stackblitz-starters-s9htwc?file=index.js

We can see for the location parsing

source: Source {
      body: 'query ($test: Int) {\n' +
        '  test(arg: $test) {\n' +
        '    ...Frag\n' +
        '    ...Frag\n' +
        '  }\n' +
        '}\n' +
        '\n' +
        'fragment Frag on Test { id }',
      name: 'GraphQL request',
      locationOffset: [Object]
    }

Which is the non unrolled variant of the fragments, just the original document. I would advise to leverage noLocation: true if the source.body is an issue for you

[marvinsemmelroth]

Thanks @JoviDeCroock for providing the example!

You're correct that I could use the noLocation-Flag which would remove the location containing the source, but doing so would require me to still inline the fragments (moving them to the root) because otherwise the parser would have no access to them, throwing an "Unknown Fragment"-Error.

[JoviDeCroock]

Yeah, but that's always been the case 😅 you can use tools like GraphQL-CodeGenerator client-preset/... but composing your fragments is normal.

Also fragments can always be composed upwards within fragments, see Relay

[marvinsemmelroth]

Currently we're using graphql-tag which is merging all referenced documents via their location.source.body into one big element (and respectively for their previous declarations)

so let say we have:

const LabelFragment = gql`
fragment LabelFragment on Label {
  label {
    de
    en
  }
}
`
const BarFragment = gql`
fragment BarFragment on Bar {
  bar
  label {
   ...LabelFragment
 }
}
${LabelFragment}
`

const FooFragment = gql`
  fragment FooFragment on Foo {
   ...Bar
    label {
      ...LabelFragment
    }
  }
  ${Bar}
  ${LabelFragment}
`

each of these fragments can be used standalone since they all have their respective sub-fragments. Each fragment is also parsed via graphql-tag.

For graphql-tag to parse the fragment they need the whole string since the graphql parser is just taking a string or a Source-Object whose constructor also needs the whole body. To what they do is merging all given location.source.body into one big string.
For our FooFragment this would be

  fragment FooFragment on Foo {
   ...Bar
    label {
      ...LabelFragment
    }
  }
  fragment BarFragment on Bar {
    bar
    label {
     ...LabelFragment
   }
  }
  fragment LabelFragment on Label {
    label {
      de
      en
    }
  }
  fragment LabelFragment on Label {
    label {
      de
      en
    }
  }

As you can see LabelFragment will already be duplicated in this small example. Now our application has a query with some huge fragments queried on multiple layers which leads to some serious duplications within the body.

So i could fix the example by just removing ${LabelFragment} from the BarFragment since FooFragment is declaring it. So its fine to use FooFragment like that. But if somewhere else BarFragment is being used it would always leave the responsibility of declaring all fragments of subfragments to the developer using the fragment.

Thats why my suggestion would be that the parser also takes already parsed definitions as input so it doesnt have to double parse all those fragments.

[JoviDeCroock]

That sounds more like an issue with graphql-tag, let me start by saying that de-duplicating things isn't the responsbility of a parser. The parser can interpret strings and as gql-tag uses a template literal they can interpolate the existing document nodes without the need to parse them. They however choose to use the doc.source.loc.body https://github.com/apollographql/graphql-tag/blob/main/src/index.ts#L125-L132 rather than merge the definitions

[marvinsemmelroth]

Is there any way to merge the definitions? I'd be happy to request this change in graphql-tag if possible. In my research I didn't find a way to provide or merge the definitions other than by doing via their source.body strings

[JoviDeCroock]

😅 Very straightforward, you might need to filter out duplicate fragments but... just the gist of it

const parsed = parse(literals[0]);
const otherDefinitions = args.flatMap(x => x.definitions)

parsed.definitions.push(...otherDefinitions)

Parsing does not execute validation so you can parse the literal without the fragments being defined in the document, later on we add them. Now graphql-tag as stated in their comment allow for 'arbitrary strings' in the interpolations, so they might need to parse when the arg is a string rather than a document but that's also possible 😅

Either way, this is the reference implementation that follows the GraphQL spec closely, we try to add as little layers as possible and optimize for readability/... you might need to take this up with graphql-tag.

[marvinsemmelroth]

At first glance it seems like a huge improvement for my case. After some further testing I think I might open an issue within graphql-tag.
Thanks a lot for you help and pushing me into the right direction! :)