Generic Types for stricter type safety, and search #239
Conversation
Entity type T in AbstractSearch, RawSearch, Search, WhereField, SchemaDefinition, Schema, and Repository classes now defaults to Record<string, any>. This change provides flexibility as it allows the underlying data structure to be a general object where keys are string types and values can be of any type.
Changed the EntityKeys type to exclude keys from EntityInternal instead of just symbol and number. Also, fixed a formatting issue with the EntityDataValue type definition.
Changed conditions in 'anyWhere' method to check for function types instead of string. Also updated instances of field identifier use to ensure type safety by casting identifier to string. This improves error messaging and overall type consistency across
|
Thanks for the contribution. It is very greatly appreciated as I have not had time for awhile to work on Redis OM. I'll try to review it sometime next week and see about getting it merged in. |
|
Only issue I'm having is the type safety with declaring Entity. Since it is Currently the IDE will still help in type safety but not warn or error if you add any key. |
There was a problem hiding this comment.
Overall this looks great and is very much appreciated. I have a couple of questions that are commented on where they occur and a couple of nits to pick regarding consistency in formatting with the imports that I'd like to address before merging this in.
First, the existing code has spaces within the curly braces on imports. Many of these have been changed—probably be automatic code stylers—to not include those spaces. I realize spacing is an arbitrary choice, but consistency matters. I went with extra whitespace because, well, because I did. 😉
The second is the use of $lib for a few imports—I think I only saw this for Entity. This is probably and automatic import things as well. However, I used relative paths throughout the codebase. I could be wrong on this call and using $lib might be better. I am happy to reconsider it. But if so I would like that as a separate PR and to be applied everywhere.
Could you please update these? Then I'll merge the PR, update the version number, regenerate the docs, and push it out to NPM?
Again, this is a great PR and I know I'm being nitpicky on the above. Thanks!
|
I'll have these resolved soon, most of these issues are mostly due to human error, I can't remember my thought process from this time. |
Take a much time as you need. I'll be offline tomorrow and through the weekend. So I'll probably not get to it until Monday. |
|
No problem, you should slap a prettierrc file in there, so contributors can follow correct formatting. |
This pull request only modifies type declarations to allow users to add a generic type to their schemas, repositories, entities, and search.
This will not cause any breaking changes as the default type for Entity is
Record<string, any>ISSUE #227