[Relay 18] Relay Resolver return type not getting derived from resolver function

[waynezhang1995]

Creating issues for Relay

Questions regarding how to use Relay and/or GraphQL

We are experimenting with Relay 18 and have noticed that the return type of a Relay resolver with RelayResolverValue is no longer being derived in the generated TypeScript type definition

For example:
Given the following relay resolver implementation

type ContactInfo = {
  address: string;
  phone: string;
};

/**
 * @RelayResolver PersonalDetails.contactInfo: RelayResolverValue
 */
export function contactInfo(personalDetailsRef: RelayResolversPostalCodeFragment$key): ContactInfo {
  const data = readFragment(
      graphql`
        fragment RelayResolversPostalCodeFragment on PersonalDetails {
          address
          phone
        }
      `,
      personalDetailsRef,
    );

  // a very dummy example
  return {...data}
}

In the generated relay artifact that references the contactInfo

import { ConcreteRequest, Query } from 'relay-runtime';
import { FragmentRefs } from "relay-runtime";

// This is missing in Relay 18
// import { contactInfo as personalDetailsContactInfoResolverType } from "../RelayResolvers";

export type RelayResolversComplexRequiredThrowQuery$variables = Record<PropertyKey, never>;
export type RelayResolversComplexRequiredThrowQuery$data = {
  readonly viewer: {
    readonly userProperties: {
      readonly personalDetails: {
        readonly contactInfo: ReturnType<typeof personalDetailsContactInfoResolverType> | null;
      } | null;
    };
  };
};

Relay used to derive the return type import { contactInfo as personalDetailsContactInfoResolverType } from "../RelayResolvers";, but this is now missing in Relay 18

Appreciate any insights. Thanks

Reporting issues with Relay

We will be using GitHub Issues for our public bugs. We will keep a close eye on this and try to make it clear when we have an internal fix in progress. Before filing a new issue, make sure an issue for your problem doesn't already exist.

The best way to get your bug fixed is to provide a reduced test case. To make reproduction simple for you, and set-up simple for Relay's maintainers, you can use Glitch:
https://glitch.com/edit/#!/remix/relay-starter-kit

You can also provide a public repository with a runnable example.

Security bugs

Facebook has a bounty program for the safe disclosure of security bugs. With that in mind, please do not file public issues; go through the process outlined on that page.

[waynezhang1995]

We are on the following:

 "react-relay": "^18.0.0",
 "relay-runtime": "^18.0.0",
"relay-compiler": "^18.0.0"

 "@types/react-relay": "^16.0.6",
"@types/relay-runtime": "^17.0.4",

Could the issue be due to the @types being outdated?"

[captbaritone]

Thanks for the report! Let me see if I can repro/fix.

[captbaritone]

I think this relates to these lines:

• relay/compiler/crates/relay-typegen/src/write.rs

  Line 191 in 87d1ef6

  // TODO: Add proper support for Resolver type generation in typescript: https://github.com/facebook/relay/issues/4772

• relay/compiler/crates/relay-typegen/src/write.rs

  Line 470 in 87d1ef6

  // TODO: Add proper support for Resolver type generation in typescript: https://github.com/facebook/relay/issues/4772

We actually have a few integration tests which currently show this broken behavior. For example:

• https://github.com/facebook/relay/blob/87d1ef64ad33543becde1af9c17444943215eb08/compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/typescript_resolver_type_import.expected#L40C35-L40C54

@drewatk do you want to take a look at this?

See also: #4772

[captbaritone]

Note: Removing those == Flow test seems to add the required imports, but also adds some unused imports which I know can be an issue for typechecking.

[Markionium]

Drew is on holiday, but I submitted a fix #4791.

I ran into this issue yesterday and realised we made a mistake by removing it, thinking it was always unused.

Removed ifs around the import statement but left the TODOs as there is still work to do.

@captbaritone might make sense to have a Typescript validation check on the generated files so CI can catch this? (That is after we fix type checking those files and remove the @ts-nocheck.)

[waynezhang1995]

@captbaritone any updates on this issue, please? Are we good to merge the fix? It is currently blocking us from upgrading to Relay 18.

[Markionium]

@waynezhang1995 it was fixed in this commit
4782743

@captbaritone can hopefully release a new version that includes it.