hybridsjs / hybrids

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

Clearing a draft causes a TypeScript error. #237

Closed Qsppl closed 4 months ago

Qsppl commented 5 months ago
import { Model, define, store } from "https://esm.sh/hybrids@8.2.11"

export interface IModel {
    prop1: number
}

export const ModelStore: Model<IModel> = {
    id: true,
    prop1: 1
}

function submit(host: IComponent) {
    store.submit(host.draft)
    host.draft = undefined
}

export interface IComponent extends HTMLElement {
    draft: IModel
}

export default define<IComponent>({
    tag: "a-component",
    draft: store(ModelStore, { draft: true }),
})

image

message:

Type 'undefined' is not assignable to type 'IModel'. ts(2322)

tsconfig.json

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "ESNext",
    "strict": true,
    "baseUrl": "./web",
    "paths": {
      // "/" is not "C:/", but is "./web"
      "/*": [ "*" ],
      // from module to their declaration
      "/modules/fingerprintjsv3.js": ["../node_modules/@fingerprintjs/fingerprintjs/dist/fp"],
      "https://esm.sh/mitt@3.0.1": ["../node_modules/mitt/index"],
      "https://esm.sh/just-intersect@4.3.0": [ "../node_modules/just-intersect/index" ],
      "https://esm.sh/bootstrap@5.3.2": ["../node_modules/@types/bootstrap/index"],
      "https://esm.sh/hybrids@8.2.11": ["@types/hybrids"],
      "https://esm.sh/colorjs.io@0.4.5": ["../node_modules/colorjs.io/types/src/index"],
      "https://esm.sh/ts-debounce@4.0.0": ["../node_modules/ts-debounce/dist/src/index"],
      "https://esm.sh/mezr@1.1.0": ["../node_modules/mezr/dist/esm/index"]
    },
    "skipLibCheck": true
  },
  "include": [ "./web" ]
}

typescript version: "5.1.6"

Qsppl commented 5 months ago

The same thing happens when assigning an identifier to a property with a model descriptor.

import { Model, ModelIdentifier, define, store } from "https://esm.sh/hybrids@8.2.11"

export interface IModel {
    prop1: number
}

export const ModelStore: Model<IModel> = {
    id: true,
    prop1: 1
}

function setModelById(host: IComponent, id: ModelIdentifier) {
    host.model = id
}

export interface IComponent extends HTMLElement {
    model: IModel
}

export default define<IComponent>({
    tag: "a-component",
    model: store(ModelStore, { draft: true }),
})

image

message:

Type 'ModelIdentifier' is not assignable to type 'IModel'.
  Type 'undefined' is not assignable to type 'IModel'. ts(2322)
smalluban commented 5 months ago

I don't have time to check every TS error (from all of your issues) - can you confirm if the problem is solved by using TS v4?

FYI: https://github.com/hybridsjs/hybrids/issues/236#issuecomment-2009222885

Qsppl commented 5 months ago

Typescript version: 4.9.5 did not solve these problems. This seems to be untyped implicit logic.

Qsppl commented 5 months ago

Update: In the first case, this is my mistake. I should have written not

function submit(host: IComponent) {
    store.submit(host.draft)
    host.draft = undefined
}

but

function submit(host: Component<IComponent>) {
    store.submit(host.draft)
    host.draft = undefined
}

But the error from the second case does not disappear even after this fix:

import { Model, define, store, Component } from "https://esm.sh/hybrids@8.2.11"

export interface IModel {
    id: number
    prop1: number
}

export const ModelStore: Model<IModel> = {
    id: true,
    prop1: 1
}

function setModelById(host: Component<IComponent>, id: number) {
    host.model = id
}

export interface IComponent extends HTMLElement {
    model: IModel
}

export default define<IComponent>({
    tag: "a-component",
    model: store(ModelStore, { draft: true }),
})
smalluban commented 5 months ago

This "was" a limitation of the TS itself, that the setter must accept the same type as a getter. I am sure, it was like this when I created definitions for TS.

Your type says:

model: IModel

But you are trying to set it to a number. As this is expected in hybrids, I am not sure if this is possible to define in TS.

EDIT: You can try with mode: IModel | number, but then you are opening many conditions when using that property, as TS claims that it might be also a number (which getting the value is not correct).

Qsppl commented 5 months ago

Hid it in a helper function:

import { Model, define, store, Component } from "https://esm.sh/hybrids@8.2.11"

function setModelDescriptorId<C extends Component<any>>(component: C, property: object) {
    let [key, value] = Object.entries(property)[0]
    /// @ts-ignore
    component[key] = value
}

export interface IModel {
    id: number
    prop1: number
}

export const ModelStore: Model<IModel> = {
    id: true,
    prop1: 1
}

function setModelById(host: Component<IComponent>, id: number) {
    setModelDescriptorId(host, { model: id })
}

export interface IComponent extends HTMLElement {
    model: IModel
}

export default define<IComponent>({
    tag: "a-component",
    model: store(ModelStore, { draft: true }),
})

But my typescript skills were not enough to write correct generics for this helper function.

Qsppl commented 5 months ago

This "was" a limitation of the TS itself, that the setter must accept the same type as a getter. I am sure, it was like this when I created definitions for TS.

Different getters and setters for interfaces are now available, but it is unclear how current types based on "mapped types" can be rewritten as interfaces.

There is currently no way to use access options in mapped types. There is an open feature request for this at microsoft/TypeScript#43826. It's marked as "Waiting for More Feedback" which means they want to hear more community feedback in favor of it before considering the possibility of its implementation. Could you write a comment there saying that this feature is necessary?