facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.37k stars 1.82k forks source link

Typescript Validation for Resolver Types #4772

Open drewatk opened 1 month ago

drewatk commented 1 month ago

Relay Resolvers currently generate a type for Flow which typechecks resolver types using a type cast:

import {User as userRelayModelInstanceResolverType} from "UserTypeResolvers";
// Type assertion validating that `userRelayModelInstanceResolverType` resolver is correctly implemented.
// A type error here indicates that the type signature of the resolver module is incorrect.
(userRelayModelInstanceResolverType: (
  id: User__id$data['id'],
  args: void,
) => LiveState<mixed>);

In typescript today, only the import statement is generated, with no implementation yet for the type assertion statement.

The assertion should be added to validate that the imported function matches the expected type. This can leverage the satisfies keyword added in Typescript 4.9.

import {User as userRelayModelInstanceResolverType} from "UserTypeResolvers";
// Type assertion validating that `userRelayModelInstanceResolverType` resolver is correctly implemented.
// A type error here indicates that the type signature of the resolver module is incorrect.
(userRelayModelInstanceResolverType satisfies (
  id: User__id$data['id'],
  args: void,
) => LiveState<mixed>);

This will depend on #4745 is fixed and enabled by default to enable typechecking in generated files. It should also include the new context type which is being implemented in #4704. I will add references to this issue in #4704 where applicable.

It will also require a minimum Typescript version of TS 4.9 or higher for consumers, which can be checked as we implement #4755. We may have to have add a feature flag to disable this for consumers of the library which do not meet this minimum standard.

@captbaritone @alloy @Markionium.

captbaritone commented 1 month ago

This sounds right to me. Thanks for writing it up! One aside to note:

We generally think these type assertions, while necessary today, lead to poor developer experience. A type error in a generated file is unintuitive as a user. However, it's needed because the docblock schema declaration must be kept in sync with the actual code.

The long term goal of deriving GraphQL schema directly from the Flow/TypeScript code will allow us to move away from these type assertions since the resolver functions themselves will be the source of truth for derived schema, there will be no need to assert that the TS types match the schema types.

I think the approach you outline here is still the right way forward in the near term, but wanted to draw this connection to how our eventual end state of generating schema directly from TS code will render the need for these type assertions (and the non ideal devX they create) obsolete.

cc @evanyeung