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

The type signature of getAll() does not permit more than 4 document keys. #126

Open
vxern opened this issue May 2, 2024 · 2 comments
Open
Labels
bug Something isn't working

Comments

@vxern
Copy link

vxern commented May 2, 2024

Describe the bug
The method is typed as accepting only 1-4 document keys, which is incorrect, given that it is generated from this specification:

[TermType.GET_ALL, 'getAll', 0, -1, 'optional']

Where I understand that -1 represents the fact that the method can take an indefinite number of arguments, which means it should support an indefinite number of document keys.

Expected behavior
It should be possible to get any arbitrary number of keys using the method, not only four at max.

@vxern vxern added the bug Something isn't working label May 2, 2024
@vxern vxern changed the title getAll() does not accept more than 4 document keys. The type signature of getAll() does not permit more than 4 document keys. May 2, 2024
@atassis
Copy link
Collaborator

atassis commented May 4, 2024

The problem is that we can't do that properly in typescript without changing an order of the arguments for the getAll, which is a breaking change. Several years ago I was thinking about making a shim for this with inverted arguments, or with providing ids as arguments. And, as you understand, got to no conclusion. I'll think about it, still, you are free to make any suggestions, I'll see them through

@vxern
Copy link
Author

vxern commented May 4, 2024

I think having the keys passable as an array is a neat solution, like so:

getAll<T>(table: RTable<T>, keys: any[], options?: { index: string; }): RSelection<T>;

I'm not too familiar with the underlying package code and how the package could deal with the keys argument being an array, but couldn't another overload such as this one be added without any breaking change? I would assume that a key (key1, key2, etc.) cannot (or at least is not expected) to be an array, even if the type is currently any, but I would leave a person more familiar with the codebase such as yourself to answer that.

Another solution could be to leverage TypeScript's variadic tuple type signatures, where TypeScript would ensure that the type of the last parameter met the spec for what the indexes parameter should look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants