Unused co-located fragment definition(s) VsCode

[tianyingchun]

Describe the bug

i have defined two fragment Channel, GlobalSettings in two individual files Channel.ts,GlobalSettings.ts

// Channel.ts
import { graphql } from '@/apollo/graphql';
export const Channel = graphql(`
  fragment Channel on Channel {
    id
    code
    token
    pricesIncludeTax
    defaultTaxZone {
      id
      name
    }
    defaultShippingZone {
      id
      name
    }
    defaultLanguageCode
    defaultCurrencyCode
  }
`);

// GlobalSettings.ts
import { graphql } from '@/apollo/graphql';

export const GlobalSettings = graphql(`
  fragment GlobalSettings on GlobalSettings {
    id
    availableLanguages
  }
`);

and i have a hooks named useGlobalSetting in ts file useGlobalSetting.ts

import { Channel } from '@/apollo/data/global-settting/Channel';
import { GlobalSettings } from '@/apollo/data/global-settting/GlobalSettings';

const globalSettingAtom = atom({
  activeChannel: undefined as FragmentOf<typeof Channel> | undefined,
  availableChannels: [] as FragmentOf<typeof Channel>[],
  availableLanguages: [] as Array<{
    name: string;
    fullName: string;
    code: LanguageCode;
  }>,
  activeLanguage: initialActiveLanguage,
  activeUILanguage: initialActiveUILanguage,
});

  const setGlobalSettings = useEventCallback(
    (globalSettings: FragmentOf<typeof GlobalSettings>) => {
      if (globalSettings) {
        setSetting((prev) => {
          const languages = normalizeLanguages(
            readFragment(GlobalSettings, globalSettings).availableLanguages
          );
          return {
            ...prev,
            availableLanguages: languages,
          };
        });
      }
    }
  );

but it give me warnings

1. Unused co-located fragment definition(s) "Channel"
2. Unused co-located fragment definition(s) "GlobalSettings"

in fact i have another file query.ts i have used these fragment

import { graphql } from '@/apollo/graphql';
import { Channel } from './Channel';
import { GlobalSettings } from './GlobalSettings';

export const queryGlobalSettings = graphql(
  `
    query globalSettings {
      globalSettings {
        ...GlobalSettings
      }
    }
  `,
  [GlobalSettings]
);

export const queryChannels = graphql(
  `
    query channels {
      channels {
        items {
          ...Channel
        }
        totalItems
      }
    }
  `,
  [Channel]
);

Reproduction

[ISSUE REPO ] (https://github.com/tianyingchun/gql.tada-issue)

gql.tada version

"@0no-co/graphqlsp": "1.10.1",
"gql.tada": "1.5.1",

Validations

• [X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• [X] Read the docs.
• [X] Follow our Code of Conduct

[JoviDeCroock]

I mean, this warning is pretty valid as you aren't using them in the component you are importing them from but they are exported. Maybe we could add a heuristic when the component you are importing it into defines no GraphQL operations of its own but it feels kinda against the fragment co-location ideology that tada is selling.

In that case if you want to not follow that methodology then you can turn off this setting. That's the picture I get from this issue but, no reproduction so very hard to say that's accurate

[tianyingchun]

yes, i agree in tsx react component it pretty good for this warning, but for .ts normally we should define somethings want to get FragmentOf<typeof xxx> to get typings , this is common requirement, we can not avoid this warning message, it just don't feel feel very well, 👍

[kitten]

@tianyingchun: You can avoid the warning message, but it's hard to tell whether this is a valid case or not from context.

Your state potentially defines references to a bunch of fragments that are then pulled together into ad-hoc state. However, the file doesn't define a single fragment that describes the shape of said state in the context of GraphQL, or in the context of globalSettingAtom’s shape.

It's obviously not trivial for us to tell you what that shape should be, but the fragments, if they’re specifically for this global state, should at least be co-located with said state, if their purpose is to be used just in globalSettingAtom (apart from being spread elsewhere)

So, indeed, this looks like the warning is correct here, as this is potentially violating the diagnostics’ rule for colocated fragments, despite not being in a component.

To put it in a different way: The globalSettingAtom consumes the two fragments, and (ignoring the atom for a moment and pretending its a component), passes this state on. But the fragments for this state consumption are not owned by that file i.e. not in that file.

[tianyingchun]

I will attach repo tomorrow

[tianyingchun]

https://github.com/tianyingchun/gql.tada-issue,

yarn install

please see warnings

1. components/selector-channel/channel-selector.tsx

3. hooks/useGlobalSetting.ts

Could you give me some suggestions? or above codes is not best best practice for gql.tada, How can we do better?

because gql.tada is very very Amazing than i used apollo before.

[JoviDeCroock]

That codebase is quite huge, was expecting a minimal reproduction 😅 that being said it highlights very well what both Phil and me assumed, it does not follow Fragment co-location.

The idea is that your page defines the query that should be dispatched and that your hook and components define their data-requirements. While in your case, all the queries and fragments are defined in a separate folder and file making them really hard to discover. Let's apply this to your channel for example, this would start embedding the useDataChannels hook as creating separate files for this is kinda redundant.

channel-selector.tsx

// your imports
const queryChannels = graphql(
  `
    query channels {
      channels {
        items {
          ...Channel
        }
        totalItems
      }
    }
  `,
  [Channel]
);

export const ChannelSelector = () => {
  useQuery(queryChannels)
  // more component stuff
}

Now if you were to have a logical UI unit that represents a channel you could use the fragment for Channel in there, if there is no logical unit for a channel there is no need to define a fragment.

I could not really find where your app starts but generally you would even hoist this up further, i.e. rather than having to define the query-boundary in your components it could be on page and ChannelSelector could export a fragment that tells the page what data it needs like

fragment ChannelSelector_Fields on Query {
  channels { items { ... } }
}

Another good article about that can be found here

[tianyingchun]

thanks for your explaination, that's very usefull for me , i totally agree indeed it does not make sense to split these into individual files, put fragment to component is good.