Skip to content

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Qsppl
Copy link
Contributor

@Qsppl Qsppl commented Mar 26, 2025

No description provided.

@Qsppl
Copy link
Contributor Author

Qsppl commented Mar 26, 2025

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.

@Qsppl Qsppl force-pushed the fix(store)-types-of-direct-methods branch from 50ced23 to 588c24f Compare April 2, 2025 19:46
@Qsppl Qsppl force-pushed the fix(store)-types-of-direct-methods branch from 588c24f to 8d1bbca Compare April 3, 2025 19:44
@Qsppl
Copy link
Contributor Author

Qsppl commented Apr 4, 2025

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.

@Qsppl Qsppl force-pushed the fix(store)-types-of-direct-methods branch from f5f3a4e to 1a4db56 Compare April 9, 2025 19:39
@Qsppl
Copy link
Contributor Author

Qsppl commented Apr 21, 2025

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.

@smalluban
Copy link
Contributor

Sure, thank you for your contribution.

@Qsppl
Copy link
Contributor Author

Qsppl commented May 26, 2025

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 },
}

@smalluban
Copy link
Contributor

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.

@Qsppl Qsppl force-pushed the fix(store)-types-of-direct-methods branch from 486e685 to 1a4db56 Compare June 4, 2025 20:37
@Qsppl
Copy link
Contributor Author

Qsppl commented Jun 17, 2025

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.

Damn, I rewrote the model definition types (Look at the types/index.d.ts), but it was fucking hard. I spent a month digging into the typing.

Everything works functionally there, but there are some cosmetic details left:

  • hiding conditional typing so that users don't get an unreadable mess in the intellisense prompts
  • TSDoc for types and comments on the code
  • backward compatibility and @deprecate for old names
  • and generally proofreading, and more careful testing

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>> :
Copy link
Contributor Author

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;
Copy link
Contributor Author

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

@smalluban
Copy link
Contributor

smalluban commented Jun 26, 2025

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.

@Qsppl
Copy link
Contributor Author

Qsppl commented Jun 26, 2025

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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants