hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.05k stars 85 forks source link

The "store(Model, { id:value })" descriptor does not work correctly. #241

Closed Qsppl closed 7 months ago

Qsppl commented 7 months ago
          Solving this error resulted in another error that I can't find.

The component with model { id: 0, ... } now receives a model, but then loses it: https://codepen.io/qsppl/pen/jORBqed?editors=1010

Originally posted by @Qsppl in https://github.com/hybridsjs/hybrids/issues/240#issuecomment-2009541659

smalluban commented 7 months ago

The model id field is always a string. You cannot change that, but you can have a storage, which requires a number - this is not a problem. Just transpose it in get and set store model methods.

However, on the "hybrids" side - in the component, store model etc.. you should use string. For example, your model type is incorrect, as the id won't be a number. You should have this:

interface IModel {
    id: string // <--- it is always a string
    content: string
}

Still, I made a fix to protect from passing numbers (it only applies to this edge case when id is 0 - as this is falsy value in JavaScript).

https://github.com/hybridsjs/hybrids/commit/faec4c67cc255b60a705cd86c2ed8ef0f4703e3b

Qsppl commented 7 months ago

Thanks for the answer.

This is not very convenient when the models are used not only by Hybrids. You have to write code like this:

interface IBaseUser { id: string | number }

interface IHybridsUser { id: string }

interface IUser { id: number }

function normalizeHybridsModel(model: IHybridsUser): IUser {
    return { ...model, id: Number(model.id) }
}

function convertToHybridsModel(model: IUser): IHybridsUser {
    return { ...model, id: String(model.id) }
}

If such a restriction on the "id" property is really necessary, then it is worth adding it to the generic of type Model:

image

export type Model<M extends { id?: string } & object> = {
    [property in keyof Omit<M, "id">]: Required<M>[property] extends Array<
        infer T
    >
    ? NestedArrayModel<T> | ((model: M) => NestedArrayModel<T>)
    : Required<M>[property] extends object
    ? Model<Required<M>[property]> | ((model: M) => M[property])
    : Required<M>[property] | ((model: M) => M[property])
} & {
    id?: true
    __store__connect__?: Storage<M> | Storage<M>["get"]
}

Then the following error will pop up: image