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

simplify user database declarations #461

Conversation

demetris-manikas
Copy link
Contributor

Resolves #459
Unfortunately this PR introduces a breaking change since the httpServer/middleware had to be adjusted as well.

@devhawk
Copy link
Collaborator

devhawk commented May 22, 2024

Can we do the unbreaking part of this work now and save the breaking part for a future 2.0 release?

@demetris-manikas
Copy link
Contributor Author

I removed the breaking changes.

@demetris-manikas demetris-manikas marked this pull request as draft May 23, 2024 17:22
@demetris-manikas
Copy link
Contributor Author

demetris-manikas commented May 23, 2024

Sorry about this.
I got carried away because all tests and linting were passing;
This must be dropped and the changes in a previous PR about reducing any usage that changed
<T extends any[]> func ( ...args T> to func (...args: unknown[])
and
<C extends UserDatabaseClient> func (TransactionContext<C>) to func (TransactionContext<UserDatabaseClient>)
were a mistake and have to be reverted.

This declaration

type Func<T extends unknown[]> = (a: string, ...args: T) => void;

allows to define an implementation like the following which declares more than it should

const FuncImpl: Func<[number]> = (a: string, b: number) =>{}

BUT

type Func2 = (a:string ...args: unknown[]) => void;

does not allow do declare

const Func2Impl: Func2 = (a: string, b: number) => {}

BUT only

const Func2Impl: Func2 = (a: string, b: unknown) => {}

which is terrible.

As I realize now the proper way to define a function that takes any number of arguments is

type Func = (a: string ...any[]) => void

and it is safe since as it only declares that this is a function that must have a string argument and can take any number of additional arguments.

I will prepare a PR later that reverts all the affected code by #448.

Sorry again.
Typescript is a pain sometimes ....

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.

Simplify UserDatabase/UserDatabaseQuery/UserDatabaseTransaction declarations
2 participants