Feature request: the Relay compiler should accept tagged literals of the form graphql(backtick ... backtick), not just graphql backtick ... backtick

[rbalicki2]

• If Relay accepted template literals of the form

graphql(`...`)

then one could ingest the persisted queries json file and generate a file like this one, and give each graphql invocation the correct type (i.e. the type of the default export of the generated file) in TypeScript.

This requires changes to the babel plugin along the lines of this

If y'all are down for this, I can make a PR real quick.

[rbalicki2]

Some context as to why I'm asking for this:

• I will have a function that takes eight generics, four query types and four pagination fragment types:

function createAuthDesktopPicker<AuthDesktopQuery, AuthDesktopPaginationFragment, ...>({ authDesktopQuery, authDesktopPaginationFragment, ... })
=> ({ isDesktop: boolean, isAuth: boolean }) => usePaginationFragmentHookType

• The reason for this is that we have issues with queries with large ASTs, and thus for each actual query, we will have four queries of the form:

graphql`
  query FooUnauthDesktopQuery {
    ...Tier1CandidatesFeedPagination_query @arguments(isAuthParam: false, isDesktopParam: true)
  }
`;
// and so on

• But the pagination fragments (because they define a query) cannot accept isAuthParam as an arg. If they do, then the shared pagination query will have a giant AST :/
• Thus, for every query + pagination fragment, we actually need four queries + four pagination fragments, a la:

const tier1AuthMobileQuery = graphql`
  query Tier1CandidatesFeedAuthMobileQuery($isDesktop: Boolean!) {
    ...Tier1CandidatesFeedPaginationAuthMobile_query
  }
`;
// the pagination fragment spreads some common fragment with `@arguments(isAuthParam: true, isDesktopParam: false)`

• The DevEx of createAuthDesktopPicker will be substantially improved if users don't have to import and provide 8 types! And if they don't provide these types, there's no way to ensure that compatible queries and pagination fragments were provided.

[rbalicki2]

Doing this without changes to Relay is blocked on microsoft/TypeScript#33304 :/

[captbaritone]

One challenge I see with this is that it presents the appearance that one could do:

const foo = `<some graphql query text>`;
const query = graphql(foo);

Or even

const foo = someDynamicStringConstruction();
const query = graphql(foo);

Obviously the compiler can't parse that in the generic case, and I don't think it's possible to have the type system (TypeScript/Flow) error on that either. I'm not even sure the compiler would be able to report an error in that case unless we make it invalid to call any function named graphql, which might be problematic. See for example this graphql-js example code: https://www.graphql-js.org/getting-started/#writing-code

[rbalicki2]

In Isograph, what we do is throw when babel encounters a call to iso if anything besides a template literal (i.e. backtick string) is passed in. We also throw if it has any interpolations (quasis).

I think that's a pretty good backstop. (It's possible to use Isograph without Babel, but that's not really encouraged. In those cases, you might have a silent failure.) This fails loudly in NextJS:

(And we could add an eslint rule to disallow this, too. I think it's rare that folks can't run eslint on their codebase.)

That being said, having to add a type parameter to get any type safety is pretty bad I think is a bit more dangerous, especially because there's no enforcement that the correct type parameter is passed :/

[captbaritone]

Some context as to why I'm asking for this: [...]

I don't know that this specific issue is common enough to merit the complexity/risk added here. But I am thinking more broadly about the possibility of having TypeScript infer types from tagged template literals. I recall you had employed some tricks in Isograph which generated function declarations for each query such that TypeScript could infer the return type? Am I remembering that correctly?

If that's possible, it feels like it could potentially justify the complexity/risk added here.

[rbalicki2]

Yeah, that's another reason I want this.

That doesn't need to be part of Relay, or developed as part of Relay, (though it can 100% be upstreamed), but the goal will be to use the persisted queries json file to generate a file along the lines of:

import {graphql: originalGraphQL} from 'relay-runtime'
import { type FooQuery } from '__generated__/FooQuery.graphql.js'

type MatchesWhitespaceAndString = ... // copy this from https://github.com/isographlabs/isograph/blob/a28a4a47b846695b98f14d58cdde0c82e7c317a6/demos/pet-demo/src/components/__isograph/iso.ts#L68-L87

export function iso<T>(
  param: T & MatchesWhitespaceAndString<'query FooQuery', T>
): FooQuery;

export function graphql(text): any {
  return originalGraphql(text);
}

• plus an eslint rule to enforce that we never import graphql from relay-runtime anymore

Anyway, this can be done as a script in userland (for us), but it certainly would be cleaner if this was generated by the compiler.

[rbalicki2]

Hey @captbaritone — any thoughts on this? I'd love to get a quick PR out if it sounds good on your end

[captbaritone]

I don't think it's worth adding the complexity to Relay compiler/babel transform/new lint rule and adding the additional possibility of overmatching on graphql functions, and additional mental overhead of there being two ways to do things if the only payoff is that some users can use it to hack around n very specific issue.

However, if we can pair this with a fundamental, built-in out-of-the-box compiler update which allows TypeScript users to avoid needing type params for things like useFragment, I'd be very interested. But I think I'd need to see much more complete outline of how we would get there (assuming it's possible).

[rbalicki2]

Oh, sorry, yeah, let me clarify my intentions. I was hoping to do this in parts, since smaller PRs are easier to ship.

• The part that requires Relay changes is the support for backtick parameters.
• As a follow up, I was going to generate the graphql overloads file. I was initially planning on doing that in userland, so as not to impose upon Relay, but if you're supportive of shipping this change in Relay, it definitely makes sense to do it in Relay proper.

If that sounds good to you, I can get started on it. Just let me know, and I'll start a separate issue where we can track progress on this.

[captbaritone]

I'd like to have a better understanding of how we will be able to enable this improved TS typing to ensure it's going to work out well before we start writing/reviewing/merging PRs.

Would it be possible to put together a small example repo with manually updated artifact files showing what you had in mind in terms of what the compiler would emit to enable the improved typing? I think that would help validate the plan a bit and also let us play around with it to get a sense of how it will play out.

For example, will this trigger any kind of bad typescript compiler performance if you have hundreds of fragments?

[captbaritone]

Cc @alloy this could be a pretty big quality of life improvement for typescript users. No need to add type params to useFragment and friends.

But we might want your help to evaluate perf implications for a large ts codebase.

[rbalicki2]

Awesome, I'll produce:

• an AST-grep search-and-replace that replaces your graphql-with-backtick calls to graphql function calls accepting backticks,
• an AST-grep search-and-replace that replaces your imports of graphql from react-relay to @magicallyTypedRelay
• a script that ingests a persistedQueries.json file and generates an overload file

@alloy et al can:

• generate a persisted queries json file and run the script, generating an overload file
• modify your tsconfig to point @magicallyTypedRelay to that generated overload file via tsconfig.compilerOptions.paths
• your app won't work at runtime, unless we also modify the babel plugin (which should be trivial, though, tbh), but it will give you a sense of the typescript perf

I don't think there's a way to avoid adding the tsconfig path, unless react-relay (or @types/react-relay is itself generated). But maybe! Who knows. The file can, of course, be automatically generated by Relay.

[captbaritone]

Does it have to be done as a single file? Is there perhaps way to achieve this using declarations such that each declaration actually lives in the fragment/operations's own file?

Maybe using merged interfaces or similar? https://www.typescriptlang.org/docs/handbook/declaration-merging.html#merging-interfaces

[rbalicki2]

Whoa, TIL. Deepseek seems to thing this is possible with interface merging:

// file 1
declare interface MyFunction {
  (x: number): number;
}
// file 2
declare interface MyFunction {
  (x: string): string;
}
// implementation
const myFunction: MyFunction = (x: any) => {
  // Implementation handling all overloads
  return x;
};

If this works, I'll do it via this

[captbaritone]

Yeah, that's what I was imagining