[Bug]: Seeing Unknown Fragment after migration to use registerFragment

[shivghai]

What's not working?

I was previously, pre rw 7, using fragments (successfully) using string interpolation. Post upgrade, there are no changes to the actual query itself, except that the fragment is no longer interpolated, and is being registered instead.

I have a GQL Fragment, F_Fragment, which is being used in a query, Q_QUERY.

UPDATE - file info Here's the code for the fragment, in fragment.ts:

import { registerFragment } from '@redwoodjs/web/apollo'

const F_Fragment = gql`
  fragment F_Fragment on F {
  field1
  field2
}`

registerFragment(F_Fragment)

UPDATE - file info The code for the query, in query.ts:

const Q_QUERY = gql`
  query QQuery {
     f {
        ...F_Fragment
     }
 }

We can no longer fetch the required data using the query, and get the Unknown fragment A_Fragment error.

The query is called in this way:

import { useQuery } from '@redwoodjs/web'

export default () => {
  return useQuery(Q_QUERY)
}

Interestingly, the graphql types are generated correctly. The query also still works with string interpolation (minus types). Any ideas here?

How do we reproduce the bug?

UPDATED
Declare and register fragment in a different file than the query

What's your environment? (If it applies)

System:
OS: macOS 13.6
Shell: 5.9 - /bin/zsh
Binaries:
Node: 20.11.1 - /private/var/folders/8y/99byt5ms31d1z8jgjfmyvcg80000gn/T/xfs-d437a37e/node
Yarn: 3.2.1 - /private/var/folders/8y/99byt5ms31d1z8jgjfmyvcg80000gn/T/xfs-d437a37e/yarn
Databases:
SQLite: 3.39.5 - /usr/bin/sqlite3
Browsers:
Chrome: 123.0.6312.59
Safari: 16.6
npmPackages:
@redwoodjs/auth-auth0-setup: 7.1.3 => 7.1.3
@redwoodjs/cli-data-migrate: 7.1.3 => 7.1.3
@redwoodjs/core: 7.1.3 => 7.1.3

Are you interested in working on this?

• I'm interested in working on this

[shivghai]

I think I see the issue.

I stored and registered the fragment separately from the query, in a different file. By the time the query itself is declared, the fragment is unavailable/hasn't been registered yet. Makes sense. So, I can get fragments/queries to work, but only if they're defined in the right order in the same file, or if somehow I can guarantee that the query variable will be declared after the fragment. This is likely the bug that @pvenable is seeing in #10120 (comment)

Is there a recommended directory structure? What is the goal for registerFragment if it doesn't help with code reuse outside of the same file? Am I missing something?

I'll give it more thought, but I assume the goal is not to have all fragments and queries to be defined in one, massive file, which is what I'd have to do right now. Of course, I could try to split this one file into multiple massive files, but as soon as there's a fragment which needs to be used in queries across files, I now have to merge these files.

Any ideas? Can the Redwood team support string interpolation as well (only thing broken there is type safety atm)? This dev ex sounds not very great

[shivghai]

Not that anyone would ever do this in production code, but here's something I tried out with initialization in the same file. I'm also not sure what to do with this info other than just dump it

Case 1: 1 ms delay before registering

This does not work (throws "Unknown Fragment"):

import { registerFragment } from '@redwoodjs/web/apollo'
const USERS_FRAGMENT = gql`
  fragment UsersFragmentTest on User {
    id
    fullName
  }
`

export const USERS_QUERY = () => {
  setTimeout(
    () => registerFragment(USERS_FRAGMENT),
    1)
  return gql`
  query UsersQuery {
    users {
      ...UsersFragmentTest
    }
  }
`}

Case 2: No delay before registering

This works:

import { registerFragment } from '@redwoodjs/web/apollo'
const USERS_FRAGMENT = gql`
  fragment UsersFragmentTest on User {
    id
    fullName
  }
`

export const USERS_QUERY = () => {
  registerFragment(USERS_FRAGMENT)
  return gql`
  query UsersQuery {
    users {
      ...UsersFragmentTest
    }
  }
`}

Case 3: 0ms delay before registering

This makes it flakey (sometimes throws "Unknown Fragment"):

import { registerFragment } from '@redwoodjs/web/apollo'
const USERS_FRAGMENT = gql`
  fragment UsersFragmentTest on User {
    id
    fullName
  }
`

export const USERS_QUERY = () => {
  setTimeout(() => registerFragment(USERS_FRAGMENT), 0)
  return gql`
  query UsersQuery {
    users {
      ...UsersFragmentTest
    }
  }
`}

[shivghai]

The abstraction layer over the InMemoryCache/Apollo fragment registry seems to be the culprit. https://github.com/redwoodjs/redwood/pull/9140/files#diff-f19decdeb6ec45e8c3e8a7a9bed807dc693a46d9fb5254bf579291c7e17fa524R322-R325.

IIUC, there may be fragments which don't make it all the way to the InMemoryCache if there are fragment registrations after RedwoodApolloProvider is instantiated. I'm by no means a pro web dev guy so please check my logic, but something here seems off

[thedavidprice]

@dthyresson has been working on this — looping him in here.

[dthyresson]

Hi @shivghai thanks for the detailed info and apologies for the bumps in using fragments -- my intention was to 1) make things easier 2) I did review the approach with folks at Apollo 3) do the heavy lifting of being able to support unions and such.

Some background:

First, I've been surprised to see that people had fragments with string interpolation working as when we saw that, the specific graphql codegen loader we used could not do interpolation, so generating types failed. This was one of the factors in moving to the client preset codegen and fragment registration. So, I didn't expect much of a breaking change than it turned out to be honestly. So apologies for that.

Second, I based the fragment design and pattern off of the approach the Guild takes here: https://the-guild.dev/blog/unleash-the-power-of-fragments-with-graphql-codegen . Specifically, "Colocate GraphQL documents with the component code. Instead of writing GraphQL operations in a dedicated file, the operations are written within the component code."

In talking to the Guild and others, it was the desire to build a "component tree" that drove the graphQL developers to come up with the concept of fragments. In fact I went back to my notes:

• fragments are one the few graphql primitives
• graphql people said fragments are why build graphql
• goal of gql is to replicate view hierarchies
• a reusable view needs reusable data for the view
• light on query high on fragment

The key here is the "view hierarchy" or as the Guild puts it:

A fragmentized component tree. Instead of writing a single GraphQL operation document per visible component or extracting the component properties from a single query operation document type, which is often cumbersome), multiple fragment definitions can be composed up to a single query operation that is sent to the server. This reduces the amount of concurrent requests

So, fragments are most often not just a way to move out parts of a query to be reusing in queries, but the goal is to collocate the data needed to render a comment with the presentation of that data.

Thus:

I stored and registered the fragment separately from the query, in a different file.

I guess was not the use case I was designing for. And I confess I didn't consider it as the pattern I was aiming for was that view hierarchy and colocation of component data.

Third, may I ask why in your examples you'd want to delay registration of the fragment? Or was that just a way to reproduce when the fragment was in a separate file?

Last: the line you point to:

  // Auto register fragments
  if (fragments) {
    fragmentRegistry.register(...fragments)
  }

That's another way fragments can be registered: You can pass them into the Apollo provider which is kinda tricky -- or use the registerFragment which adds them individually.

So -- to answer the question: "Is there a recommended directory structure? What is the goal for registerFragment if it doesn't help with code reuse outside of the same file? Am I missing something?"

The goal was to let developers create the view hierarchies and collocate the data with the component. This lets Apollo manage the cache for each fragment really nicely. In addition, it will let us implement upcoming features such as fragment masking which is in RFC here: apollographql/apollo-client#11666

Hope this extra background helps. If there's something I can do to make the experience better, let me know. In the mean time, I'll ask the Guild and others about the "keeping queries all in one file using fragments" pattern and get their thought on how and when to use that.

[shivghai]

Hi @dthyresson - appreciate the thoughtful response. I have a couple of clarifications, and some explanation of how we've set up our fragments.

Third, may I ask why in your examples you'd want to delay registration of the fragment? Or was that just a way to reproduce when the fragment was in a separate file?

Yep just to reproduce different files. Ignore that.

So, fragments are most often not just a way to move out parts of a query to be reusing in queries, but the goal is to collocate the data needed to render a comment with the presentation of that data.

We do indeed use fragments for moving out parts of a query so it can be reused. From the first line of the Apollo GraphQL docs on fragments.

A GraphQL fragment is a piece of logic that can be shared between multiple queries and mutations.

From a later point in the same doc:

Colocating fragments
The tree-like structure of a GraphQL response resembles the hierarchy of a frontend's rendered components. Because of this similarity, you can use fragments to split query logic up between components, so that each component requests exactly the fields that it uses. This helps you make your component logic more succinct.

This is just one use case of fragments, which is not mutually exclusive to code reuse/logic sharing, which is now mostly broken in RedwoodJS unless if the fragments are registered before queries are defined (i.e. same-file-for-everything-approach). IIUC, we could still have "local" fragments per component to have better colocation while maintaining our code reuse fragments overall.

If we go back to Apollo Client v2 docs:

There are two principal uses for fragments in Apollo:
Sharing fields between multiple queries, mutations or subscriptions.
Breaking your queries up to allow you to co-locate field access with the places they are used.

One of these principal uses is now broken in RedwoodJS.

So, fragments are most often not just a way to move out parts of a query to be reusing in queries, but the goal is to collocate the data needed to render a comment with the presentation of that data.

I don't know if that's true, to me, it seems like that is just one use case of fragments which helps with code maintainability.

I see the colocation benefit here - it's great. But with Redwood's current approach, I also see an increase in verbosity for the moving-out-parts-of-a-query use case and a significant decrease in code maintainability. At this point it's not even helpful to use fragments since reuse is broken.

We've gone for simplicity and speed over performance benefits of colocation (again, they shouldn't be mutually exclusive IIUC, but I may be wrong), and this heavily breaks our codebase.

In the mean time, I'll ask the Guild and others about the "keeping queries all in one file using fragments" pattern and get their thought on how and when to use that.

Just a clarification - that's not exactly how we do things. We have web/src/fragments/user which includes the user fragments for example, which is then used in any queries related to users by importing the fragment and using it in the required query using string interpolation. Our data is heavily relational, and having to redefine fragments every time we want a new component with a different view of the data will be extremely painful.

What I meant by "keeping queries all in one file using fragments" is that would be what I'd have to resort to, given the current limitations.

If there's something I can do to make the experience better, let me know.

Bringing back string interpolation would be great, since that worked well for us pre RW7!

Let me know if I can answer any other questions on our usage!

[shivghai]

@dthyresson What would work best would be to bring back string interpolation, or to somehow hoist certain files/directories in Redwood so we can guarantee fragment registration first. If there's already a way to do that, that'd be perfect

[dthyresson]

@dthyresson What would work best would be to bring back string interpolation, or to somehow hoist certain files/directories in Redwood so we can guarantee fragment registration first. If there's already a way to do that, that'd be perfect

That’s the mystery to me — I never saw string interpolation in fragment code working. It always threw an error when doing type/code generation.

Unless a newer version of the code file loader now supports it. I’ll look into that.

[dthyresson]

FYI this said that the loader did not permit interpolation https://stackoverflow.com/questions/62749847/graphql-codegen-dynamic-fields-with-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

btw I’m not trying to say it didn’t work for you but I’m just so surprised since I even have some Slack messages with other Core Team members from last year who ran into this issue when building some apps and had to help fix.

[pvenable]

That stack overflow question appears to be about interpolating fields into fragments rather than including fragment definitions into other fragments or queries, which was the way to reference other components' fragments prior to the fragmentRegistry. See my first code block in #10120 (comment) for an example of the latter. As mentioned in that issue, the steps described there did work in Redwood 6.5.1, and that's how we always used fragments from Redwood 1.x until v7 (though we did start migrating some of our fragments to Apollo's fragment registry before Redwood added the native integration).

Personally I think the fragmentRegistry is an improvement overall, but I do think there is a potential footgun in that (as far as I know) there's nothing guaranteeing at runtime that a particular fragment has indeed been registered before referencing it from elsewhere. For example, it seems one could (accidentally) reference a fragment that has been code-split into a separate bundle or whose registration code has otherwise not been imported. (That could be what's going on in this issue.)

This may just be the nature of the thing, and when following "normal" patterns like referencing fragments of imported child components it does seem safe to assume that registration will have occurred due to having directly imported the component file that registers that fragment. But I could see cases -- like including a fragment into a mutation* to ensure the cache is updated appropriately -- that could fail depending on whether the bundle that registers a given fragment has been loaded. 🤔 If my theory is correct, the answer (for now?) may just be "don't do that!", but I haven't confirmed this specific problem.

(*: This seems like a reasonable use case to me, but I don't think I've done this much, and perhaps it's a mistake. 🤷‍♂️)

[shivghai]

@pvenable

For example, it seems one could (accidentally) reference a fragment that has been code-split into a separate bundle or whose registration code has otherwise not been imported. (That could be what's going on in this issue.)

Definitely what's going on right now in this issue

But I could see cases -- like including a fragment into a mutation* to ensure the cache is updated appropriately -- that could fail depending on whether the bundle that registers a given fragment has been loaded.

We currently do this, part of the reason why we share fragments across multiple queries/mutations

[dthyresson]

I think I’ll have to create a small demo app and try to recreate the issue and then work to see if I can restore the 6.5.1 behavior.

Can I ask if in your toml you have any of the GraphQL settings enabled like fragments https://redwoodjs.com/docs/cli-commands or trusted documents?

The fragments config will try to swap out the gql function with one that supports possible types.

For my test app, I’d turn those off.

[shivghai]

Thanks for looking into this @dthyresson !

Pre-upgrade, I didn't have any gql related settings set up
Post upgrade, I had

[graphql]
  fragments = true

to get the fragment registry working

I'm happy to hop on a call to discuss and post findings here if that's helpful, unless if you prefer async in which case that's fine too

[shivghai]

Demo app should be easy and I imagine it's top of mind for you, but I can outline what it could look like in case that's helpful:

If you're starting with Redwood 6.x:

web/src/fragments/users.ts -> declare a fragment for user info here
web/src/components/UserCard -> use the fragment in a query here, using string interpolation. You can also add a mutation here which uses the same fragment
web/src/components/UserChip -> use the same fragment here, using string interpolation

[dthyresson]

I'm happy to hop on a call to discuss and post findings here if that's helpful, unless if you prefer async in which case that's fine too

I might take you up on that :)

To clarify, this is the behavior I was familiar with in v6 and before and what the entire register fragment DX intended to make "easier" -- or so I hoped.

• All is happy because the fragment and using the fragment are all in the same gql so it can know what BookParts is
  

• Not happy as the fragment is unknown

• Squiggles not happy but the fragment did actually work (little surprised frankly as not what I expected)
  

but the type check shows the error:

fragment-test main % yarn rw tc
✔ Generating the Prisma client...
✔ Generating types
src/components/BooksCell/BooksCell.tsx:26:5 - error TS2554: Expected 1 arguments, but got 2.

26   ${bookPartsFragment}
       ~~~~~~~~~~~~~~~~~


Found 1 error in src/components/BooksCell/BooksCell.tsx:26

Again, little surprised by this as not what expected.

[dthyresson]

Interesting ... manually importing graphql pluck seems to help:

No squiggles and types and tc passes:

ragment-test main % yarn rw g types && yarn rw tc
Generating...

- .redwood/schema.graphql
- .redwood/types/mirror/api/src/directives/requireAuth/index.d.ts
- .redwood/types/mirror/api/src/directives/skipAuth/index.d.ts
- .redwood/types/mirror/api/src/services/books/index.d.ts
- .redwood/types/mirror/web/src/pages/BooksPage/index.d.ts
- .redwood/types/mirror/web/src/pages/FatalErrorPage/index.d.ts
- .redwood/types/mirror/web/src/pages/NotFoundPage/index.d.ts
- .redwood/types/mirror/web/src/components/BooksCell/index.d.ts
- .redwood/types/includes/web-routesPages.d.ts
- .redwood/types/includes/all-currentUser.d.ts
- .redwood/types/includes/web-routerRoutes.d.ts
- .redwood/types/includes/api-globImports.d.ts
- .redwood/types/includes/api-globalContext.d.ts
- .redwood/types/includes/api-scenarios.d.ts
- .redwood/types/includes/api-test-globals.d.ts
- .redwood/types/includes/web-test-globals.d.ts
- .redwood/types/includes/web-storybook.d.ts
- api/types/graphql.d.ts
- web/types/graphql.d.ts

... done.

✔ Generating the Prisma client...
✔ Generating types
fragment-test main % 

Note: I think this was brought up by the gql global earlier, but I just made the connection.

Going to investigate this since it appears to at the very least be a possible workaround -- though it means importing pluck everywhere.

[dthyresson]

@shivghai and @pvenable I think I may have found the culprit and even why I may not have seen it when working on fragments as the change to help support Trusted Documents aka Persisted Operations changed the type of the global gql.

The generic type we had made for TD support seems to be incompatible with the graphql-tag:

With this change, TS and GraphQL seem to be happy:

I need to do some tests that TD still works, but I think you have two workarounds:

1. import graphql-tag on own
2. patch the global.web-auto-imports.d.ts in web node modules dist (web/node_modules/@redwoodjs/web/dist/global.web-auto-imports.d.ts) to

// ...
declare global {
  const gql: (
    source: string | TemplateStringsArray | readonly string[],
    ...args: any[]
  ) => DocumentNode;
//...

It's that ...args that we didn't have that seems to be the root of the expecrtect 1 but got 2 etc.

I also been to test that this is ok with the fragments setting turns on -- but I think I am on the right track.

Since TD was implemented after, and our e2e/CI/test suite used the collocated fragments pattern this got missed unfortunately.

[shivghai]

Awesome, thanks for looking into this @dthyresson . I'll wait to upgrade till we have a fix for this in RW, but this would get us all the way there!

[shivghai]

@dthyresson is this something i can help with? just want to make sure this can be resourced

[shivghai]

@dthyresson - just following up here.