Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript Validation for Resolver Types #4772

Closed
drewatk opened this issue Aug 14, 2024 · 1 comment
Closed

Typescript Validation for Resolver Types #4772

drewatk opened this issue Aug 14, 2024 · 1 comment

Comments

@drewatk
Copy link
Contributor

drewatk commented Aug 14, 2024

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
Copy link
Contributor

captbaritone commented Aug 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants