-
Notifications
You must be signed in to change notification settings - Fork 87
fix(store) types of direct methods #288
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
base: main
Are you sure you want to change the base?
Conversation
Hi, while doing documentation on direct store methods I came across a problem - the documentation does not match the types. The correction of the types took several days, this is a very complex API. This week I will finish writing the specification in TypeScript and then it will be possible to safely fix the typing bug. After that I will be continue writing TSDoc. |
50ced23
to
588c24f
Compare
588c24f
to
8d1bbca
Compare
Type tests in "test\types\store\model-definition.ts" have been broken all this time) We need to separate cases into positive and negative to avoid repeating the problem. I'll do this in the next few days. |
f5f3a4e
to
1a4db56
Compare
Hi, I'm changing jobs now, so I haven't had time to work on Hybrids in the last few weeks. I'll be back to work on this in a couple of weeks. |
Sure, thank you for your contribution. |
Next week I'll get a new job and will have time to work on the library again. I had time to solve an old problem with tree subset stores. After much research, it turned out that if a structure with an index key "{ [x: string]: ... }" flies into Model<> - the entire typing simply breaks. I wrote tests for this case and fixed the situation with the "StripIndex" helper. In parallel, I finished working on another problem - for fields with a combined type - for example "(string | number)". I'll leave here the code that solves both problems because I don't have time to commit it now: type StripIndex<T> = {
[
K in keyof T as K extends string
? string extends K ? never : K
: K extends number
? number extends K ? never : K
: K
]: T[K]
};
type Model<T> = {
[K in keyof Omit<StripIndex<T>, "id">]: Transform<T[K]>;
};
type IsStringLike<T> =
string extends T ? true :
String extends T ? true :
NonNullable<T> extends string ? true :
NonNullable<T> extends String ? true :
false;
type IsNumberLike<T> =
number extends T ? true :
Number extends T ? true :
NonNullable<T> extends number ? true :
NonNullable<T> extends Number ? true :
false;
type IsBooleanLike<T> =
boolean extends T ? true :
Boolean extends T ? true :
NonNullable<T> extends boolean ? true :
NonNullable<T> extends Boolean ? true :
false;
type IsStrictObjectLike<T> =
...;
type Transform<Value> =
IsNumberLike<Value> extends true ? number :
IsStringLike<Value> extends true ? string :
IsBooleanLike<Value> extends true ? boolean :
Value extends object ? Model<Value> :
never;
// ########################################################################################################
// tests for trees
interface ICountrySubdivisionsNode extends Record<string, number | ICountrySubdivisionsNode> { }
interface ICountrySubdivisionsSchema extends Record<string, number | ICountrySubdivisionsNode> { }
interface IUSSubdivisions extends ICountrySubdivisionsSchema {
id: number;
alabama: number
states: {
alabama: number
}
}
const subvisions: Model<IUSSubdivisions> = {
alabama: 0,
states: { alabama: 1 },
} |
Nice. In general, I more often think about rewriting TypeScript types, as they become less and less readable to me. I don't know if TypeScript can fit the hybrids structure well. |
486e685
to
1a4db56
Compare
Damn, I rewrote the model definition types (Look at the Everything works functionally there, but there are some cosmetic details left:
I'll put this in a separate request because there are a lot of changes there |
*/ | ||
[key in keyof Omit<OmitIndexes<TModel>, "id">]-?: | ||
| (TModel[key] extends object ? | ||
PickIndexes<TModel[key]> extends { [key: string]: infer ItemType } ? Referable<StoreRecord<ItemType>> : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part needs to be checked for distributivity, it seems to me that not all possible results should be returned here, but only the result of the first comparison, which is wrong
* // ModelDefinition[] would match Model | ||
* } | ||
*/ | ||
export type Model = StrictObject<{ id?: ModelIdentifier }> & NonModelDefinition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AbstractModel maybe. Or ModelSignature because it is not a contract but a helper for matching types
I appreciate your dedication to creating 100% accurate type definitions, but I would rather make the typings simple to read and use, rather than focus on being error-proof. You also export them all (I expect for testing), but then users might use them by accident. I expect no one else than you is heavily using TS at the moment (me included), so I don't care much about it. Just be aware that at some point in time, I may clean them up and make them simpler. I am not sure if this is worth spending that much of your time. |
Regarding exports - thank you very much for the comments, i will split the typing as public API and private API and export only public. In order for the typing to remain supported, I will try to describe the code as clearly as possible with comments, examples and illustrations. Well, as for wasting time - from the current tasks, in addition to typing, there is also documentation, but the documentation is written directly on top of the types, so without correct and consistent typing, chaos begins in my documentation. Thanks for correcting things like exports, some things are really not obvious to me here, don't be shy to write if I'm doing something wrong |
No description provided.