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

Support type overrides in config #60

Closed
silasdavis opened this issue May 8, 2020 · 5 comments
Closed

Support type overrides in config #60

silasdavis opened this issue May 8, 2020 · 5 comments

Comments

@silasdavis
Copy link
Contributor

silasdavis commented May 8, 2020

Allow override of type mapping form config by adding types section that takes a map of postgres type names to Type:

{
  "transforms": [ ... ],
  "types": {
    "int:": {
      "name": "WholeNumber",
      "from": "./my-pkg/whole-number"
    },
    "bytea": {
      "name": "Uint8Array"
    },
    "shipping_status": {
      "name": "ShippingStatus",
      "enumValues": ["Shipped", "Pending"]
    }
  },
}

This would support the currently supported AliasedType, ImportedType, and EnumType definitions and apply them as overrides into the TypeMapping passed to TypeAllocator

Some issues:

  • Relative import resolution - we probably need to accept imports rooted at srcDir in config and convert them to relative imports in code. There are probably some corner cases to consider when emitting generated code outside of the source tree (is that even allowed?)
@silasdavis
Copy link
Contributor Author

silasdavis commented May 29, 2020

A related thought on this - do we want to get involved in actually parsing values? Currently we are just relying on mapped types being assignable from whatever the database driver provides. In the case of pg this is mostly strings by default.

If we want to support composite types it seems like we will need parsing, we might also want to parse things like dates, intervals, and uuids.

How best to handle this? We can generate (or import) parsers/serialisers and use them to wrap PreparedQuery (and the input/output types) so you'd have a raw query version you could call and a parsed version (that calls the raw query).

We could then describe types by a pair of functions. At this point we may want to extend my Type representation with a proper Typescript AST (presumably the Typescript compiler API, no idea how friendly this is). Then we can pull the param/result types from the serialisation/parser function definitions:

type ParseableType<T> = { 
 to: AST<T => string>; 
 from: AST<string => T>
}

export Type<T> = NamedType | ImportedType | AliasedType | EnumType | ParseableType<T>;

@silasdavis
Copy link
Contributor Author

Hm, well the generic types I used above imply we would have the target type available at generation type which we presumably would not if it is custom or if it depends on the SQL, but vaguely like that.

@adelsz
Copy link
Owner

adelsz commented May 29, 2020

I think adding parsers is a step in the right direction.
Initially I planned for PgTyped to be a mostly compile-type library, but can see how it can be much more useful with an embedded client (node-postgres, and then maybe our own client based of wire).

I like the idea of wrapping PreparedQuery and injecting parsers/serializers code there. We might even get away with some light wrapping because pg supports injecting type parsers like this:

const queryConfig = {
  text: 'SELECT * from some_table ...',
  values: ...,
  types: {
    getTypeParser: () => val => val,
  },
}
client.query(queryConfig);

I am not sure I understand why a generic param is needed in ParseableType. It should be ok if it looks like this?:

type ParseableType = { 
 parserCode: string; 
 serializerCode: string;
 typeDefCode: string;
}

We still have full control over which types we emit for query result and params. Wdyt?

@JesseVelden
Copy link
Contributor

JesseVelden commented Jan 3, 2022

Hi @adelsz , I’ve also looked into the implementation of this feature, but I found some obstacles in the way:

Parsing & different Postgres clients

If we want to override pg’s type parser, we can do that using the method you mentioned above. However this will only work for the pg client. I am actually not sure how to implement type parsers in other clients, but we could simply direct users, wanting to use our types parsers to this issue, explaining their use case.

Diverting from pg’s type parser/ serializer

Right now if users are actually using it with a different client than pg, it must strictly follow the parsing and serializing rules of pg in order to have the correct types.
That’s why I propose making a different @pgtyped/types package, that gives us the following benefits:

@pgtyped/types package

  • Being explicit, and clear about parsing, serializing and type generating separate from any client.
  • Might be nice, when you really want to make PgTyped a full postgres client

I think it's nice to use this as an opt-in feature first, in order not to break any user setup with different Postgres clients.

Overriding types

I also propose using cosmicconfig and cosmiconfig-typescript-loader for config files in the cli package. This way we can keep existing support for config.json, but also allow config to be specified using a TypeScript, which can be really nice for overriding. I like that more than specifying an import function by strings.

You can let know what you think, and I can give it a shot at an implementation

@adelsz
Copy link
Owner

adelsz commented Jan 28, 2023

Basic type overrides have now been added. Imported type overrides are in the work as part of #480

@adelsz adelsz closed this as completed Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants