taiga-family/ng-polymorpheus

[FEATURE] Use more idiomatic Typing

qortex opened this issue · 5 comments

Typing using object or {} is linted by eslint as banned (rule @typescript-eslint/ban-types).
But current typing in the library makes it hard to avoid.

Would you consider changing to more precise types such as unknown where possible (e.g. I find TemplateRef<unknown> handy in my code and do not see drawbacks to that), or other choices where it applies?

If so, I could give it a shot and make some proposals.

Example: tring to use PolymorpheusContent<unknown> fails because Type 'unknown' does not satisfy the constraint 'object'.

Is there any specific reason I don't see for forcing inheritance from object?

Technically, template context has to be object, because when you write template like this:

<ng-template let-value let-index="index">
  {{index}} - {{value}}
</ng-template>

it takes $implicit and index keys from context object. So context indeed must follow those constraints. But polymorpheus is not just about templates and any type can work for functions and components so we might consider loosening the constraints.

However I'm surprised about type ban, can you elaborate on that?

Looking closer at the code, I think my initial conception was misleaded, it is indeed normal that too wide unknown should not be accepted.

Still, the ban-types rule points out an interesting point: the default {} seems not to convey the expected meaning.

I learned the subtleties by reading:

eslint suggests Record<string, unknown> which seems to be a good candidate to replace default {} in our case?

export declare type PolymorpheusContent<C extends Record<string, unknown> = Record<string, unknown>> = ...

In PolymorpheusComponent, I'm not sure having T extends object is really necessary since it's then used in Type<T>? What does it check? I didn't find material on what extends object actually enforces.

Also this advice on the same comment:

Avoid the object type, as it is currently hard to use due to not being able to assert that keys exist.
See microsoft/TypeScript#21732.

Fixed in 4.0.1