Skip to content

Conversation

@sep2
Copy link

@sep2 sep2 commented Jan 7, 2026

Omit will not work, a common interface ISql for Sql and TransactionSql is introduced.

microsoft/TypeScript#41362

`Omit` will not work, a common interface `ISql` for `Sql` and `TransactionSql` is introduced.

microsoft/TypeScript#41362
@porsager
Copy link
Owner

porsager commented Jan 7, 2026

Won't that include all properties that aren't valid for TransactionSql ? I think the proper way is to make an explicit definition of TransactionSql instead of inherit. Perhaps better to use Type instead of Interface.

@sep2
Copy link
Author

sep2 commented Jan 7, 2026

Only actual properties are included in the interfaces. Properties not in the runtime trx object are moved to sql only. The super type contains only shared properties that could be found in both runtime objects.

Object.keys(sql) [
  'types',         'typed',
  'unsafe',        'notify',
  'array',         'json',
  'file',          'parameters',
  'largeObject',   'subscribe',
  'CLOSE',         'END',
  'PostgresError', 'options',
  'reserve',       'listen',
  'begin',         'close',
  'end'
]
Object.keys(trx) [
  'types',   'typed',
  'unsafe',  'notify',
  'array',   'json',
  'file',    'savepoint',
  'prepare'
]

type could work but that's potentially a breaking change, with interface the changes are minimized.

I used sql/trx extensively with this PR in my local project, haven't found any type errors so far. Nothing in my project needs to adapt.

@mattbishop
Copy link

It would be good to add a unit test, like what is common in DefinitelyTyped.

@porsager
Copy link
Owner

porsager commented Jan 8, 2026

To prevent the same hazard as previous PR, can some more people with typescript knowledge approve this?

@abrahamguo
Copy link

abrahamguo commented Jan 9, 2026

@porsager I opened the original issue (#1143), and I can confirm that this looks good to me. I've also tested this in my project, and it resolves the issue that I was seeing.

@sep2's line of thinking, of moving all shared properties (sql.types/trx.types, sql.unsafe/trx.unsafe, and so on) AND the call signature (sql()/trx()) to a new shared interface used by both sql and trx is the right approach.

@abrahamguo
Copy link

The only nitpick I would have is that since there's already an existing interface called Sql, creating a new interface ISql is a poor name, because it's unclear what the difference is between Sql and ISql.

A more descriptive name might be BaseSql.

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 this pull request may close these issues.

4 participants