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

Improve typing customization #480

Closed
nick-keller opened this issue Jan 27, 2023 · 6 comments · Fixed by #488
Closed

Improve typing customization #480

nick-keller opened this issue Jan 27, 2023 · 6 comments · Fixed by #488

Comments

@nick-keller
Copy link
Contributor

nick-keller commented Jan 27, 2023

Once again, amazing work on your part @adelsz!

Here are a few ideas for improving typing with pgtyped. I'm ready to do it but I wanted to have everyone's aproval on the idea before going further.

Complex mappers

Today it supports basic TS types:

{
  "typesOverrides": {
    "date": "string",
    "json": "any"
  }
}

It would be nice to be able to specify imported types in the fashion of what GraphQL codegen does.

{
  "typesOverrides": {
    "date": "dayjs#Dayjs"
    "myEnum": "./types/enums.ts#MyEnum"
  }
}

❓ What should be the root of the relative paths?
❓ What should be the specs of the format?

Different mapping for parameters

It would be nice to type parameters and return values differently, even if they are of the same PG type.
For instance Decimal are returned as string by PG, so all select should be typed as such, but when inserting, updating, or using it in a where clause PG accept both a string or a number.

  • The default typing should reflect that
  • The config file should also allow to override those types as well

❓ What should the config file look like exactly?
❓ What is the behavior expected if you only override one of the two?

Customize generated types

Today is generates interface and enum, it would be nice to choose between type interface and even class and between enum and string union.

types as well

❓ What should the config file look like exactly?

@adelsz
Copy link
Owner

adelsz commented Jan 27, 2023

Thanks for volunteering for this @nick-keller!

Here are some of my thoughts, please feel free to challenge & discuss:

What should be the root of the relative paths?

Paths should be absolute starting from the pgTyped config directory. We can consider relative paths later if there is demand.

What should the config file look like exactly?

It would be great to have type mapping conform to the following snippet. I am split on the "parameter" and "return" key names, maybe "parameterType" and "returnType" would be better.

{
  "typesOverrides": {
    "date": "dayjs#Dayjs"
    "myEnum": {
      "parameter": "./types/enums.ts#MyEnum",
      "return": "string"
  }
}

What is the behavior expected if you only override one of the two?

Overriding just the parameter or return type should leave the other one set to default value.

Today is generates interface and enum, it would be nice to choose between type interface and even class and between enum and string union.

That would be useful, but I see it as independent to the type overrides since it affects the types of the whole return/parameter sets and not individual fields, so suggesting to discuss it as a separate feature.

@nick-keller
Copy link
Contributor Author

To sum it up we could have:

For imports

  • types that start with ./ are imports
  • types that contain a # are imports
  • imports that start with ./ are relative path to the config file, others are absolute imports
  • imports can have 0 or 1 #, otherwise it throws
  • an import that does not contain a # imports default
  • an import that ends with as .+ is aliases
  • all imports of the same package are merged

For return vs parameter

  • if you specify a string, both return and parameter are set
  • if you specify only one key, the other keeps its default built in value

@adelsz
Copy link
Owner

adelsz commented Jan 29, 2023

All looks good. Alias imports are a bit too much in my opinion, so I suggest to not add them.
On an implementation note, we might want to introduce local per-query type overrides in the future, so lets keep that in mind when designing the implementation for these global type overrides.

@nick-keller
Copy link
Contributor Author

Actually alisases are very useful to solve naming conflict. From my experience with GraphQLCodegen (from which this spec is inspired) it was very handy to get out of tricky situations.
Plus the cost seams very low once imports are implemented anyway.
We can start without and see what we do from there ;)

@jakewhelan
Copy link

Would really love to use this feature, any idea when it might be published to npm?

@nick-keller
Copy link
Contributor Author

The PR #494 was merged 2 days ago, it should be good

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.

3 participants