[Bug?]: Errors including graphql fragments on v7

[pvenable]

What's not working?

(First described in #9969 (comment) before I was convinced it was a framework issue.)

I noticed errors with including graphql fragments while attempting to upgrade an app from 6.5.1 to 7.0.7.

In cases like this:

const fragment = gql`
  fragment ExampleFragment on Whatever {
    id
    ...SomeOtherComponentFragment
  }

  ${SomeOtherComponent.fragments.foo}
`;

...I see errors like Expected 1 arguments, but got 2 on the line ${SomeOtherComponent.fragments.foo}.

I also see this error in my editor when navigating to the gql tag definition in node_modules/@redwoodjs/web/dist/global.web-auto-imports.d.ts:

This sort of fragment inclusion worked fine in Redwood 6, and the gql tag reference then resolved to node_modules/graphql-tag/src/index.ts, so it seems something has changed.

Since I can immediately reproduce this on a new Redwood 7.0.7 app, it seems to be a framework issue rather than anything to do with the app I was trying to upgrade.

Note that there is a potential workaround in the form of migrating all "legacy" fragments to the fragment registry, but for users with many such fragments it may be a fairly involved change to upgrade to v7. I also haven't confirmed this workaround.

---

How do we reproduce the bug?

# Generate a typescript project
yarn create redwood-app rw-7-gql-tag-issue --typescript
cd rw-7-gql-tag-issue/

# Get some SDL to work with
yarn rw g scaffold UserExample

Add these two fragments (or just paste this in) to web/src/pages/UserExample/UserExamplesPage/UserExamplesPage.tsx:

import UserExamplesCell from 'src/components/UserExample/UserExamplesCell'

const fragment1 = gql`
  fragment Foo on UserExample {
    id
    name
  }
`

const fragment2 = gql`
  fragment Bar on UserExample {
    id
    ...Foo
  }

  ${fragment1}
`

const UserExamplesPage = () => {
  return <UserExamplesCell />
}

export default UserExamplesPage

Check your editor or run yarn rw tc to see the type error:

yarn rw tc

Expected result: No error output from yarn tsc; fragment interpolation typechecks successfully

Actual result:

✔ Generating the Prisma client...
✔ Generating types
src/pages/UserExample/UserExamplesPage/UserExamplesPage.tsx:16:5 - error TS2554: Expected 1 arguments, but got 2.

16   ${fragment1}
       ~~~~~~~~~


Found 1 error in src/pages/UserExample/UserExamplesPage/UserExamplesPage.tsx:16

Note: Following the equivalent steps on Redwood 6.5.1 works as expected.

---

What's your environment? (If it applies)

Redwood 7.0.7

Are you interested in working on this?

• I'm interested in working on this

[dthyresson]

@pvenable In v7, RedwoodJS added more complete GraphQL Fragment support - see the new documentation here: https://redwoodjs.com/docs/graphql/fragments

The reason one cannot:

const fragment2 = gql`
  fragment Bar on UserExample {
    id
    ...Foo
  }

  ${fragment1}
`

Is that the codegen we use doesn't allow for string interpolation.

And, with yarn rw setup graphql fragments we modify how the gql function works to automatically support the way Apollo requires union types to be configured in order to use them with fragments.

Could you try your fragments with the docs above and let me know if they work ok?

Thanks.

[pvenable]

I see, so this is a known breaking change, and it's expected that we would need to migrate our ~200ish GraphQL fragments to the new fragment registry API in order to upgrade to v7? I don't think I saw this called out in the release notes or upgrade guide, but I will migrate our fragments if that's what it takes.

However, I'm currently getting this output from the new setup command, so I'll have to investigate later whether the error matters:

% yarn rw setup graphql fragments
✔ Update Redwood Project Configuration to enable GraphQL Fragments
✔ Generate possibleTypes.ts
✔ Import possibleTypes in App.tsx
✖ You cannot call "get" on a collection with no paths. Instead, check the "length" property first to verify at least 1 path exists.
You cannot call "get" on a collection with no paths. Instead, check the "length" property first to verify at least 1 path exists.

Actually -- it looks like the error is occurring on the step "Add possibleTypes to the GraphQL cache config", so I can presumably do that manually and move forward.

Thanks for the clarification, and please let me know if I've misunderstood anything. I'll update this issue when I've had a chance to migrate our fragments.

[pvenable]

I've migrated all of our fragments to the new registerFragment API and things seem to be working smoothly in my testing so far. Thanks for clarifying that this is the only supported way to use fragments now.

I'm uncertain whether to close this issue as I feel like I'm still technically reporting an unannounced breaking change, but I'm not sure if I glossed over it somewhere or whether there's even much to be done about it at this point.

[pvenable]

Update: I have had to disable a couple of tests in our codebase with weird failures like "No fragment named FooFragment" (but with legitimate fragment names) for now because they fail quite (though not 100%!) consistently after the migration.

The same components and fragments work fine in the app, and out of hundreds of tests only these two are breaking in this way, but since it's just these two I suspect a potential edge case with the fragment registry in tests, possibly related to Cells as the failing tests each happen to render components that render Cells that contain queries with fragments from other components that the Cells import and render. But this could all be a red herring.

I don't have a repro for this and I likely won't be able to investigate it any further for some time.

[dthyresson]

I've migrated all of our fragments to the new registerFragment

Glad to hear -- and thanks so much for doing the upgrade -- I imagine yours is by far one of the larger apps using fragments.

still technically reporting an unannounced breaking change

That's on me -- I honestly didn't think it was a breaking change -- in fact I thought the new fragment support made fragments possible. In the past we had many reports that using interpolation of the fragments in the queries was breaking GraphQL codegen and the Guild confirmed that the code-loader codegen plugin we used didn't support string interpolation.

" GraphQL Codegen and the tooling it uses doesn’t support string interpolation, and it’s not going to add support for it, because in order to get the final string, we’ll need to load, compile and look for that exported identifier. Doing that will make the setup of codegen much more complex - because of the many flavours in the JS ecosystem."

So, I was actually surprised that it had been working for you.

The fragment support in v7 also auto generates the possibleTypes that Apollo Client must have if you use union is your SDL.

Again, apologies for the confusion -- it was unintentional.

possibly related to Cells as the failing tests each happen to render components that render Cells that contain queries with fragments from other components that the Cells import and render. But this could all be a red herring.

That is interesting. I don't think I tested that case.

I was aiming for some patterns that resembled: https://the-guild.dev/blog/unleash-the-power-of-fragments-with-graphql-codegen

Appreciate your work and testing this in depth - I'm glad that Redwood can now offer some more UI and data pattern renderings with fragments.

[pvenable]

In the past we had many reports that using interpolation of the fragments in the queries was breaking GraphQL codegen and the Guild confirmed that the code-loader codegen plugin we used didn't support string interpolation.

Interesting! We've been using fragments in the more "legacy" manner described in my initial post since Redwood 1.x! I don't think we did anything fancy to make it work, but it would be interesting to read about the problems others have had here.

The only problem I think we really had before was some seemingly nondeterministic in-editor error messages about unrecognized fragments on the "spread" lines (...FooFragment), which we lived with for quite a while, but I believe that went away recently once we configured documents: in graphql.config.js.

I was aiming for some patterns that resembled: https://the-guild.dev/blog/unleash-the-power-of-fragments-with-graphql-codegen

Thanks for linking this. This is similar to what we do; however:

• We aren't using useFragment yet (but I've been meaning to look into it).
• This is (somehow) the first time I've seen "fragment masking", which looks like it may help solve a pain point we've had with "accidentally" accessing data from other components' fragments.
  • Can we do this today with Redwood, or would that require further integration? This is the first time I've seen client-preset, so I'm not sure yet whether this needs to be added directly to our app or whether it's now baked into Redwood (or whether it needs to be).

Appreciate your work and testing this in depth - I'm glad that Redwood can now offer some more UI and data pattern renderings with fragments.

Me too -- thanks for the integrated fragment support. I like what you've done here and I look forward to further improving our fragment usage. 👍

[dthyresson]

• This is (somehow) the first time I've seen "fragment masking

@pvenable it's neat, no?

The Guild's graphql tools etc useFragment can support that -- but Redwood uses Apollo doesn't ... yet ... as this just popped up today which I'll need to investigate: apollographql/apollo-client#11666

RFC: Data masking with fragments #11666 ^^^

Since RW already uses useFragment: see:

redwood/packages/web/src/apollo/fragmentRegistry.ts

Line 53 in 23a9fac

return apolloClient.useFragment({

Hopefully thing will go smoothly:

"
We [Apollo] will allow data masking to be opted into globally by introducing a new option to ApolloClient. Enabling this option would automatically turn on data masking for every query that uses the client instance.

new ApolloClient({ 
  // This name is subject to change
  dataMasking: true
})

We [Apollo] will recommend that new users and applications opt into this feature immediately as the default
"

[jerelmiller]

@dthyresson glad to see this discussed here 🙂. I'll make sure we stay in touch as we work through the feature to make sure the paved road is smooth for Redwood as well.

[shivghai]

seeing the same breaking change - need to migrate about ~200 fragments as well. will take a look at registering fragments

[shivghai]

Tried the upgrade. Seeing other issues now. Can't get fragments to work. Opened #10322