Crank: Typescript support for props in Component type

[monodop]

This library does a decent job with typescript, but it'd be really nice to have some support for strongly typing props in the Component type, and it might also be nice to have the same in Context. One use-case for this is to be able to use a Component<MyExpectedProps> in a function signature or config option. Another use-case would be defining type-safe higher order components. I can't see a way to get the prop types to propagate down properly as things are, but I could just be missing something.

Here's a concrete example of something I might want to do. I know I could pass the thing down with this.set but then I still don't get type-safety. With this method I could also require every route to have the prop.

import {createElement, Context, Component} from '@bikeshaving/crank';

export type RouteProps = {
    currentRoute: Route;
}

export type Route = {
    name: string;
    index: Component<RouteProps>;
}

export type RouterProps = {
    routes: Route[];
};

function getCurrentPath(): string[] {
    return new URL(window.location.href)
        .hash
        .substring(1)
        .split('/')
        .map(p => p.trim())
        .filter(p => p);
}

function getMatchingRoute(routes: Route[], currentPath: string[]): Route | null {
    const p = currentPath[0];
    const matches = routes.filter(r => r.name == p);
    if (matches.length > 0)
        return matches[0];
    return null;
}

export function* Router(this: Context, {routes}: RouterProps) {
    let currentPath = getCurrentPath();
    const listener = () => {
        currentPath = getCurrentPath();
        this.refresh();
    }
    window.addEventListener('popstate', listener);
    try {
        while(true) {
            const route = getMatchingRoute(routes, currentPath);
            if (route) {
                const C = route.index;
                yield <C route={currentRoute} />;
            }
            else {
                yield <div>Not found</div>
            }
        }
    }
    finally {
        window.removeEventListener('popstate', listener);
    }
}

[shirakaba]

This is all guesswork without having opened up a code editor, but I have a hunch:

Current situation

In @brainkim 's TodoMVC example, the following signature appears:

function* Header(this: Context): Generator<Element> {
    // ...
}

If you check index.ts, you'll see that Element is a generic:

export interface Element<TTag extends Tag = Tag> {
    [ElementSigil]: true;
    readonly tag: TTag;
    props: Props;
    key?: unknown;
}

... Props, however, is currently a non-generic:

export interface Props {
    "crank-key"?: Key;
    children?: Children;
    [name: string]: any;
}

Possible solution

If Element took generic Props like so (note that I've moved the ElementProps generic to the front so that users don't have to write a TTag in):

export interface Element<ElementProps extends Props = Props, TTag extends Tag = Tag> {
    [ElementSigil]: true;
    readonly tag: TTag;
    props: ElementProps;
    key?: unknown;
}

... then I believe we could probably explicitly-type elements like so:

function* Header(this: Context): Generator<Element<MyExpectedProps>> {
    // ...
}

Note: of course it would be even better if users didn't have to explicitly write out the return signature in the first place. I doubt that TypeScript could simply infer this all, but TS does have a habit of pleasantly surprising. Maybe a next step would be to see whether anything could be done to improve inference.

---

While we're here, I'm not a fan of adding the index signature [name: string]: any; to Props, as it kills all type-safety. Perhaps introduction of this generic would give the necessary expressiveness to allow that to be removed.

[brainkim]

I agree that Crank could do with some stronger typings!

I’ve tried to leverage TypeScript as best I can, but there are some things that just seem difficult to type. For instance, yield expressions can evaluate to a DOM node, a string, an array of both, or a promise of any of the above types, depending on what exactly is rendered. I’ve also been hesitant to add type parameters to types because these parameters become part of the actual API, and having to change them later would result in breaking changes. That said, I agree wholeheartedly that stronger typings would be good, especially for people who care about that sort of stuff and I will make sure to look at each typing to make sure it isn’t too much of a lift for loosy-goosy TypeScript people like me who love to sprinkle any into my codebases. So my thoughts:

1. Typing Component props
   Yes we should definitely do this. Also Component is really just a type, and you can define it in your own codebase if you need it to be strongly typed thanks to structural typing. My one suggestion is maybe provide a default parameter of any. I know, I know, this is just how I roll.
2. Typing Context
   Yes we should definitely do this. Again, a default parameter of any might be appreciated especially because it’s mostly redundant type information:

interface GreetingProps {
  name: string;
}

function Greeting(this: Context<GreetingProps>, {name}: GreetingProps) {
}

The component is always going to have a props parameter because that’s just how function components work in TypeScript. I strongly suggest a default type param of any here for convenience. Maybe if Crank becomes popular enough we can have this just be an iterable/async iterable of the first parameter but that is a pipe dream and likely never to happen.

1. Typing Elements
   Not sure about this one. I think createElement and JSX should be responsible for type checking props, not the Element type. I’m already kinda iffy on the Props type, because children really can be anything and although I hate render props as children I don’t want to limit that pattern with types.

[monodop]

I can update my PR to make sure the prop typings are optional anywhere they'd be used by a user. Do we really want to default to any? or is {} sufficient?

I wasn't sure if the Element type needed to have TProps or not. I need to check and see what the compiler is going to complain about, because it seems to be a little different than what my editor does. Personally, I think that should be the main determining factor on whether or not Element is typed.

One annoying thing for me is that I have to define props twice, once to define this and once to define props. I really think it might be more important to type Context though, since that defines the typing of the iterators. However it might kind of just "work out" if you only type the props and then reuse the same variables. Seems kind of flimsy to me though. I'll have to play more with that with the compiler. Maybe what I'll do is add a set of test cases just for checking type safety and we can see how TypeScript reacts to this proposal in different situations.

[brainkim]

One annoying thing for me is that I have to define props twice, once to define this and once to define props. I really think it might be more important to type Context though, since that defines the typing of the iterators. However it might kind of just "work out" if you only type the props and then reuse the same variables. Seems kind of flimsy to me though.

Thank you for experimenting with this. I agree that it can feel flimsy, and I want to address these concerns with a type parameter, but I do want the default to be as ergonomic as possible. That way, we can please both the strongly and loosely typed crowd, and if the strongly typed crowd has concerns they can write eslint rules to force people to explicitly double-define types.

[monodop]

I don't think the strongly typed crowd wants to define it twice either. I'll have to mess around with it and see if there's anything I can do with Typescript to make it easier to use. For example, if we could use arrow functions then you could simply write your components like this:

const Component: FunctionComponent<{message: string}> = (this, props) => {
    let i = 0;
    for ({message} of this) {
        if (++i > 2) {
            return <span>Final</span>;
        }

        yield <span>{message}</span>;
    }
};

To me, this is a very elegant solution (comparatively), except that arrow functions can't support this.

[monodop]

This works I guess...

const Component: GeneratorComponent<{message: string}> = function* (props) {
    let i = 0;
    for (const {message} of this) {
        if (++i > 2) {
            return <span>Final</span>;
        }

        yield <span>{message}</span>;
    }
}

Edit: it was actually a GeneratorComponent

[brainkim]

@monodop you don’t sound happy about it though haha.

[monodop]

I think if I could get all of these working I'd be satisfied.

type MyProps = {
    message: string;
};

// Works
const MyFunctionComponent: Component<MyProps> = function (props) {
    return <div></div>;
}

/*
Type '(this: Context<MyProps>, props: MyProps) => Generator<any, any, undefined>' is not assignable to type 'Component<MyProps>'.
  Type 'Generator<any, any, undefined>' is not assignable to type 'string | number | boolean | Element<Tag, any> | GeneratorComponent<MyProps> | PromiseLike<Child> | FunctionComponent<MyProps>'.
    Type 'Generator<any, any, undefined>' is not assignable to type 'FunctionComponent<MyProps>'.
      Type 'Generator<any, any, undefined>' provides no match for the signature '(this: Context<MyProps>, props: MyProps): MaybePromiseLike<Child>'.ts(2322)
*/
const MyGenerator: Component<MyProps> = function* (props) {
    let i = 0;
    for (const {message} of this) {
        if (++i > 2) {
            return <span>Final</span>;
        }

        yield <span>{message}</span>;
    }
}

// Works
const MyAsyncComponent: Component<MyProps> = async function(props) {
    return <div></div>;
}

/*
Type '(this: Context<MyProps>, props: MyProps) => AsyncGenerator<any, any, any>' is not assignable to type 'Component<MyProps>'.
  Type 'AsyncGenerator<any, any, any>' is not assignable to type 'string | number | boolean | Element<Tag, any> | GeneratorComponent<MyProps> | PromiseLike<Child> | FunctionComponent<MyProps>'.
    Type 'AsyncGenerator<any, any, any>' is not assignable to type 'FunctionComponent<MyProps>'.
      Type 'AsyncGenerator<any, any, any>' provides no match for the signature '(this: Context<MyProps>, props: MyProps): MaybePromiseLike<Child>'.ts(2322)
*/
const MyAsyncGenerator: Component<MyProps> = async function* (props) {
    let i = 0;
    for await (const {message} of this) {
        if (++i > 2) {
            return <span>Final</span>;
        }

        yield <span>{message}</span>;
    }
}

// Works
const MyRegularComponent: Component = <div></div>;

/*
Type '"Hello World"' is not assignable to type 'Component<any>'.ts(2322)
*/
const MyEvenMoreRegularComponent: Component = 'Hello World';

[brainkim]

I’m not sure why that’s happening and I think it’s mostly a consequence of function unions not working as we would expect: In other words, (() => number) | (() => string) is not the same as (() => number | string). Maybe the best thing to do in this situation is to use GeneratorComponent<Props>?

Again, thank you for investigating this stuff.

[monodop]

Ok, I think I have it all working in #51, but there's some super weird behavior from typescript.

This works:

type CF<TProps, TReturns> = (
    this: Context<TProps>,
    props: TProps,
) => TReturns;
export type FunctionComponent<TProps = any> = CF<TProps, MaybePromise<Child>>;

but this doesn't:

export type FunctionComponent<TProps = any> = (
    this: Context<TProps>,
    props: TProps,
) => MaybePromiseLike<Child>;

Edit: figured it out - had MaybePromiseLike in the second version and I'm apparently blind

[brainkim]

Fixed in #51 and to be deployed soon.

[brainkim]

Shipped in 0.1.3