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

Expose the PostgresError type for TypeScript #99

Merged
merged 1 commit into from
Dec 13, 2020

Conversation

akheron
Copy link
Contributor

@akheron akheron commented Aug 1, 2020

With current index.d.ts it seems to be impossible to get hold of the PostgresError type. It would be useful for e.g. signatures of error handling functions.

Fix by exposing the type but not the class.

Copy link
Contributor

@Minigugus Minigugus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can get rid of the Impl suffix, which differs from the actual implementation by the way 😉

types/index.d.ts Outdated
@@ -63,7 +63,7 @@ interface JSToPostgresTypeMap {
[name: string]: unknown;
}

declare class PostgresError extends Error {
declare class PostgresErrorImpl extends Error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
declare class PostgresErrorImpl extends Error {
declare class PostgresError extends Error {

types/index.d.ts Outdated
@@ -94,6 +94,7 @@ type UnwrapPromiseArray<T> = T extends any[] ? {
} : T;

declare namespace postgres {
type PostgresError = PostgresErrorImpl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type PostgresError = PostgresErrorImpl
type PostgresError = typeof PostgresError

types/index.d.ts Outdated
@@ -312,7 +313,7 @@ declare namespace postgres {
<T extends HelperSerializable, U extends (keyof (T extends any[] ? T[number] : T))[]>(objOrArray: T, ...keys: U): Helper<T, U>;

END: {}; // FIXME unique symbol ?
PostgresError: typeof PostgresError;
PostgresError: typeof PostgresErrorImpl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PostgresError: typeof PostgresErrorImpl;
PostgresError: PostgresError;

@akheron
Copy link
Contributor Author

akheron commented Aug 2, 2020

I don't think this is right. I need the type of PostgresError instances. typeof PostgresError gives the type of the constructor function.

@akheron
Copy link
Contributor Author

akheron commented Aug 2, 2020

I think the easiest way would be to move class PostgresError { ... } inside the namespace, but I guess it was left outside on purpose?

@Minigugus
Copy link
Contributor

I don't think this is right. I need the type of PostgresError instances. typeof PostgresError gives the type of the constructor function.

Indeed, sorry 😆. Maybe a type PostgresErrorType = PostgresError; before the declare namespace, and type PostgresError = PostgresErrorType after? 😐

I think the easiest way would be to move class PostgresError { ... } inside the namespace, but I guess it was left outside on purpose?

Yes, because in this case it becomes possible to access the class constructor from the package exports, which is wrong: it is only accessible on the sql tag.

@akheron akheron force-pushed the expose-postgreserror-type branch from dbd0135 to cae6ebf Compare August 3, 2020 17:38
@akheron
Copy link
Contributor Author

akheron commented Aug 3, 2020

Maybe a type PostgresErrorType = PostgresError; before the declare namespace, and type PostgresError = PostgresErrorType after? 😐

Fixed like this ^

@porsager
Copy link
Owner

I guess this is good to merge - right @Minigugus ?

@porsager porsager merged commit 4d5fdc1 into porsager:master Dec 13, 2020
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.

3 participants