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

We should split reader.ts into several artifacts #28

Closed
rbalicki2 opened this issue Feb 21, 2024 · 2 comments
Closed

We should split reader.ts into several artifacts #28

rbalicki2 opened this issue Feb 21, 2024 · 2 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@rbalicki2
Copy link
Collaborator

rbalicki2 commented Feb 21, 2024

Problem

  • Currently, reader artifacts contain: the generated TypeScript __param type, the generated __outputType type, and the reader AST.
  • If a resolver is unreachable from an entrypoint, we do not need to generate the reader AST. But we do need to generate the types. So, we generate unneeded reader ASTs.
  • If the resolver is never selected by another resolver and is not an entrypoint, we do not need to generate the output type. So, we generate some unneeded output types.
  • However, we need to define a __param type for all user-defined resolvers, because it is used by iso.ts. We currently generate unneeded __param types for __refetch artifacts (though, seemingly not for magic mutation fields.)

Note: the output and param types that we generate that are not needed are not as big of a deal as the unneeded reader AST, because:

  • output types are easy to create
  • __refetch fields are rarer
    So, if not generating the output and param types adds complexity, we can punt on it.

Solution

  • So, instead, we should split reader.ts into reader.ts (containing the reader AST and reader artifact), param_type.ts and output_type.ts.

Details

  • Type/field/param_type.ts should import the output_type of all selected resolvers and export the param type
  • Type/field/output_type.ts should contain the ResolverReadOutType (e.g. export type Query__HomeRoute__outputType = (React.FC<ExtractSecondParam<typeof resolver>>);) and nothing else.
  • Type/field/reader.ts should define the reader AST. It should import the user-defined exported resolver function (i.e. export const pet_stats_card = iso(...)). It should import the param and output type. It should define and export the artifact.

Follow ups:

  • We should only generate types, and not reader ASTs, for unreachable client fields
  • Maybe this has some implication for __refetch fields. That seems to work correctly now, but it would be worth double checking whether we can simplify things.
  • I'm hoping that this results in cleaner code, which will make it easier to do things like asynchronously import JavaScript (see Split entrypoint.ts artifact into two artifacts: query text and normalization AST #33)
@rbalicki2 rbalicki2 added the good first issue Good for newcomers label Feb 21, 2024
@rbalicki2 rbalicki2 changed the title We should generate separate reader.ts and types.ts artifacts We should split reader.ts into several artifacts Mar 20, 2024
@ismajl-ramadani
Copy link
Contributor

Can I take this?

@rbalicki2
Copy link
Collaborator Author

Closed by d629a5b

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

No branches or pull requests

2 participants