Minor typescript organization

[thebearingedge]

• toJSON() of JSONValue returns JSONValue type
• TTypes defaults to JSToPostgresTypMap
• JSToPostgresTypeMap is in postgres namespace

The JSONValue type already includes recursive type references, so we may as well guarantee that JSON.stringify() will get a JSONValue back.

Having to always include {} when referencing Sql<{}> is annoying.

This exposes the type parameter of the Sql interface for extension by other libraries that would like to extend it.

[porsager]

Would you mind taking a look @Minigugus ?

[thebearingedge]

@Minigugus Thank you for the review!

[thebearingedge]

@Minigugus Sorry, I believe that the build script ran due to the prepare script, but I am not accustomed to committing build artifacts so I didn't explicitly add it.

I'm pretty sure I botched synchronizing my fork with upstream, hence the rebase. I am going to attempt to clean things up this morning.

[Minigugus]

@thebearingedge Do you think you could change PostgresTypeList to Record<string, postgres.PostgresType> too? It's quite similar to the fix for JSToPostgresTypeMap so it is relevant to fix PostgresTypeList here too? #448 would be solved by this issue then

[thebearingedge]

@Minigugus yes, I should be able to take care of that 👍

[thebearingedge]

@Minigugus Ok! I think we are all set 🔥 ⌨️ 🔥

[porsager]

Is this ready to merge once conflicts have been resolved @Minigugus ?

[porsager]

Would any of you mind resolving the current conflicts ? :)

[thebearingedge]

Yes I will tackle it sometime this week 👍

[thebearingedge]

@porsager thank you, the conflict has been resolved :salute:

[thebearingedge]

Hi @porsager @Minigugus I believe we are all set, but I'm not really sure why CI failed, it seems to have completed successfully on my own CI

https://github.com/thebearingedge/porsager-postgres/actions/runs/3079235630

[akatechis]

Would this PR fix the error type "string" does not exist? I'm using postgresjs with the following code example:

import postgres from 'https://deno.land/x/[email protected]/mod.js';

const sql = postgres<PGTypes>({
  host: 'localhost',
  port: 5432,
  database: 'postgres',
  user: 'postgres',
  password: 'postgres',
});

interface PGTypes {
  string: 'text';
  datetime: 'timestamp';
  integer: 'int';
  [name: string]: string;
}

But when I run it, I get the following error, and something tells me I'm not passing the correct JSToPostgresTypeMap type parameter to the postgres factory function...

error: Uncaught (in promise) PostgresError: type "string" does not exist
    const error = Errors.postgres(parseError(x))
                         ^
    at ErrorResponse (https://deno.land/x/[email protected]/src/connection.js:761:26)
    at handle (https://deno.land/x/[email protected]/src/connection.js:468:6)
    at data (https://deno.land/x/[email protected]/src/connection.js:312:9)
    at https://deno.land/x/[email protected]/polyfills.js:140:32
    at Array.forEach (<anonymous>)
    at call (https://deno.land/x/[email protected]/polyfills.js:140:18)
    at success (https://deno.land/x/[email protected]/polyfills.js:100:11)

If there's any documentation on what to pass here, I would gladly accept a link so I can read up on how to pass a correct generic interface

[thebearingedge]

Hi @akatechis, The types option will need to be an object, like this example under Custom Types.

https://github.com/porsager/postgres#custom-types

your interface PgTypes, will need to have a separate key for each custom type you want to add.

[akatechis]

Thanks, @thebearingedge, i'll look into that. Does that require me to give type definitions for basic types like string as well, or is that just for custom types? The table I'm trying to query is just two string columns, created by the following:

sql`create table if not exists ${sql(tableName)} (id string not null primary key, data string)`

I would assume I don't need to define how to handle strings. Is that correct?

Edit: Also, apologies for hijacking the comment thread of a merged PR. It just seemed related to the issue I'm having. I'm happy to open a separate issue/discussion if you'd prefer

[thebearingedge]

@akatechis no worries 😄

regarding your custom string type. for that you will need to define it as a domain, as i do not believe that postgres has it. there is a text type though.

create domain string as text;

https://www.postgresql.org/docs/current/sql-createdomain.html

the custom type settings for the postgres package are there mainly to let you pre-process and post-process values transferring to/from your JavaScript code. so you might have something like this (just a random example)

const types = {
  string: {
    to: 25, // the PG oid for "text"
    from: [25],
    serialize: s => s.toLowerCase(), // lowercase before storing in PG
    parse: s => s.toUpperCase()  // uppercase before using in JS
  }
}