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

Sql typing no longer works with interface types #410

Closed
kkorenius opened this issue Sep 12, 2022 · 11 comments
Closed

Sql typing no longer works with interface types #410

kkorenius opened this issue Sep 12, 2022 · 11 comments
Labels

Comments

@kkorenius
Copy link

Trying to use an interface type with an SQL query, for example:

interface Person {
  id: number,
  name: string,
};

const query = sql<Person>`
  SELECT id, name
  FROM person
`;

const onePerson = await connection.one(query);

Expected Behavior

The code above should compile correctly.

Current Behavior

TypeScript compiler gives the following error:

TS2344: Type 'Person' does not satisfy the constraint 'QueryResultRow'.
  Index signature for type 'string' is missing in type 'Person'`.

The code works if the interface is replaced with a type alias.

Possible Solution

The code was working before release 30.4.2. The problem has been probably caused due to adoption of MixedRow

@kkorenius kkorenius added the bug label Sep 12, 2022
@gajus
Copy link
Owner

gajus commented Sep 13, 2022

This feature was removed in v31.

I think v30.4.2 was an accidental release.

Can you give 30.4.3 a try?

@kkorenius
Copy link
Author

Unfortunately, the same error with v30.4.3.

@gajus
Copy link
Owner

gajus commented Sep 14, 2022

Released v30.4.4 to npm

@gajus gajus closed this as completed Sep 14, 2022
@gilamran
Copy link

@gajus This is still an issue, it's impossilbe to work with interfaces and sql
I tried all these versions: 30.4.2 30.4.4 and the latest 31.2.4
All show the same error:
image

@gajus
Copy link
Owner

gajus commented Sep 30, 2022

Assuming this worked before, downgrading further will eventually find a version that works. It should be v30.4.4 though.

However, given that these are no longer supported, their use is highly discouraged.

@gilamran
Copy link

@gajus what isn't supported? interfaces?

@gajus
Copy link
Owner

gajus commented Sep 30, 2022

Interface support has been removed. All static types are getting removed in favor of Zod in future versions.

https://github.com/gajus/slonik#runtime-validation

@gilamran
Copy link

That's BIG... maybe too big

@gajus
Copy link
Owner

gajus commented Sep 30, 2022

See long discussion in this thread

#364

and this article

https://contra.com/p/gkOQlbLq-validating-postgre-sql-query-results-using-runtime-checks

@mmkal
Copy link
Contributor

mmkal commented Oct 1, 2022

@gajus I'm a bit surprised #364 is the reference point for how this dramatic decision was made. That thread, in my opinion, establishes that it would have been unnecessary to remove plain interface support for the zod feature. Then interface support was just removed as a follow up, without any external review or further opportunity for comment by me or others who have contributed to slonik over the years, as far as I could tell.

@gilamran I am on the lookout for a more suitable library for my team and will likely not maintain @slonik/migrator or @slonik/typegen once we have stopped using slonik. There are a few contenders, so happy to chat about it if you're thinking along the same lines. I'm on discord with the same username.

Edit: edited for tone. I'm sorry if that came off unpleasant, I'm very grateful for this library which has been working well in production for us for years. I'd still love to keep working with it, but what we need is a sql client for typescript and slonik is now a sql-zod-client. I did get a bit frustrated at the idea that I was part of that decision, rather than someone who actively resisted it as not necessary, so I'm sorry if I came off ungrateful or rude. 🙏

@gilamran
Copy link

gilamran commented Oct 1, 2022

@mmkal I totally with you on this. Removing support for interfaces on some minor version is not acceptable in my opinion.
Moving this library to a runtime validation (without any option to opts out) is not what I am looking for.
I've also started looking for alternatives, not doing so will get me stuck on version 30.4.4 forever.

p.s. I don't see your comment as rude. Any contributor's opinion should be heard. This is open source.

@mmkal what's your discord name and 4 digits?

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

No branches or pull requests

4 participants