Add TypeScript type

[whs]

No description provided.

[whs]

I also have types for svelte-state-renderer ready, however it depends on @pyoner/svelte-typescript which is probably not yet the community's standard?

Here it is:

declare module 'svelte-state-renderer' {
	import {Renderer, Parameter, Router} from 'abstract-state-router';
	import Component from 'dummy.svelte';

	export interface Parameters {
		target?: HTMLElement;
		anchor?: HTMLElement | null;
		props?: {};
		methods?: {};
	}

	type ComponentClass = typeof Component

	export type InputType = ComponentClass["constructor"] | {
		component: ComponentClass["constructor"],
		options: Parameter,
	}

	export type SvelteRouter = Router<InputType, ComponentClass>;

	export default function makeSvelteStateRenderer(params: Parameters): Renderer<InputType, HTMLElement, ComponentClass>;
}

The intention is that the return type of createStateRouter(makeSvelteStateRenderer({})) is aliased to SvelteRouter so that users don't need to write out the generics every time.

[TehShrike]

Awesome, thanks!

Do you think you could add some kind of automated test for the types? It doesn't have to be comprehensive for now, but something using tsd or at least node-ts to get it started.

[whs]

I found out that the EventEmitter3 types require EventEmitter3 4.0 to be able to typecheck event handlers.

Is it ok for me to bump? The only relevant breaking changes is in v3.0.0 and considering that we export the EventEmitter interface we might need to release as semver-major as well.

[TehShrike]

I think as long as we tossed a setMaxListeners noop function on the emitter, we could update the event emitter library without a breaking change in abstract-state-router.

[whs]

Just a quick update, I'm still waiting on tsdjs/tsd#45 , seems that adding types to the root will make it check node_modules as well.

[TehShrike]

I bumped some dependencies in ebd33d9 which fixed that particular issue.

Is there anything left to do?

[whs]

I don't have anything else to add, so let's merge it

[ArtskydJ]

If eventemitter is considered part of the api, then there is one more breaking change. The interface of EventEmitter.prototype.listeners() has changed slightly. Any outside consumers using the exists param can no longer use it.

- emitter.listeners(eventName, exists)
+ emitter.listeners(eventName)

As far as breaking changes go, this is about as minor as it gets.

primus/eventemitter3@2e19b9c#diff-168726dbe96b3ce427e7fedce31bb0bc

[whs]

I think it is better just to bump semver-major? I think it is better than monkeypatching the libraries we use to emulate compatibility.

[sw-clough]

I don't have time to review this .d.ts file thoroughly, but I did quickly compare it to a version I have been using locally, and I noticed a few differences. The differences noted are based upon my interpretation of the source code and readme.

1. type Parameter accepts any values, but really it can only be string | number | null
2. stateName can be null in the go function

Below is a dump of my version. I think it has some good features that could be incorporated in this version, like splitting the query string parameters out to their own types.

However, I only know the basics of Typescript, so my version will be lacking in other ways. For example, I don't know how to inform TS that the data object added to the state was the same one received by the resolve function. Looking at this code, it seems adding the <DataType> to the interface might allow that functionality, so I will now go and test that theory!

declare module "abstract-state-router" {
    import { EventEmitter } from "eventemitter3"

    export default StateProvider
    function StateProvider(
        makeRenderer: IMakeRenderer,
        rootElement: Element,
        stateRouterOptions?: StateRouterOptions
    ): StateProviderEmitter

    export interface IMakeRenderer {
        (stateRouter: StateProviderEmitter): IRenderer
    }

    export class StateRouterOptions {
        pathPrefix?: string
        router?: any
        throwOnError?: boolean
    }

    export interface IRenderer {
        render(
            context: RendererRenderContext,
            callback: IResolveCallack
        ): DomApi
        reset(
            context: RendererResetContext,
            callback: IResolveCallack
        ): void | DomApi
        destroy(domApi: DomApi, callback: IResolveCallack): void
        getChildElement(domApi: DomApi, callback: IResolveCallack): Element
    }

    class RendererRenderContext {
        template: StateTemplate
        element: Element
        content: StateResolveContent
        parameters: QueryStringParameters
    }

    class RendererResetContext {
        domApi: DomApi
        content: StateResolveContent
        template: StateTemplate
        parameters: QueryStringParameters
    }

    type DomApi = any

    export interface IResolveCallack {
        (err: any): void
        (err: Falsy, content: StateResolveContent): void
        redirect(
            stateName: string,
            stateParameters?: QueryStringParameters
        ): void
    }

    export class StateProviderEmitter extends EventEmitter {
        addState(state: State): void
        go(
            stateName: string | null,
            stateParameters?: QueryStringParameters,
            options?: { replace?: boolean; inherit?: boolean }
        ): void
        evaluateCurrentRoute(
            stateName: string,
            stateParameters?: QueryStringParameters
        ): void
        makePath(
            stateName: string | null,
            stateParameters?: QueryStringParameters,
            options?: { inherit?: boolean }
        ): string
        getActiveState(): { name: string; parameters: QueryStringParameters }
        stateIsActive(
            stateName: string,
            stateParameters?: QueryStringParameters
        ): boolean
    }

    export class State {
        name: string
        route: string
        defaultChild?: string | (() => string)
        data?: StateData
        template: StateTemplate
        resolve?: (
            data: StateData,
            parameters: QueryStringParameters | any,
            callback: IResolveCallack
        ) => any
        activate?: (context: StateActivateContext) => void
        querystringParameters?: QueryStringParameterKey[]
        defaultQuerystringParameters?: QueryStringParameters
        defaultParameters?: QueryStringParameters
    }

    export class StateActivateContext extends EventEmitter {
        domApi: DomApi
        data: StateData
        parameters: QueryStringParameters
        content: StateResolveContent
    }

    type StateData = {
        [name: string]: any
    }

    type StateTemplate = any

    type StateResolveContent = any

    type QueryStringParameterKey = string
    type QueryStringParameterValue = string | number | null
    export interface QueryStringParameters {
        [QueryStringParameterKey: string]: QueryStringParameterValue
    }

    type Falsy = false | 0 | "" | null | undefined
}

Also, because it was mentioned, below is my version of svelte-state-renderer.d.ts. This requires a svelte.d.ts file too, which is at the bottom.
Note: My setup doesn't have typescript support within the svelte templates. I'm not sure if OP's method does? These files allow me to use typescript in the files that set my routes, and then within the resolve functions.
PS: I copied a bunch of these files from somewhere.

declare module "svelte-state-renderer" {
    import { IMakeRenderer } from "abstract-state-router"

    interface DefaultOptions {
        data?: any
        props?: any
    }

    export default function SvelteStateRendererFactory(
        defaultOptions?: DefaultOptions
    ): IMakeRenderer
}

svelte.d.ts

declare module "*.svelte" {
    export default Svelte
}

declare class Svelte {
    constructor(options: { target: Element; data?: any })

    get(name?: string): any
    set(data: any): void

    on(
        eventName: string,
        callback?: (event?: any) => any
    ): () => { cancel: () => any }

    fire(eventName: string, event?: any)

    observe(
        name: string,
        callback: (newValue?, oldValue?) => any,
        options?: { init?: boolean; defer?: boolean }
    ): () => { cancel: () => any }

    oncreate(target): void

    ondestroy(): void

    destroy(): void
}

declare class ISvelte<T> extends Svelte {
    get(): T
    get(name: string): any

    set(data: T): void
}

[TehShrike]

Thanks for the input @sw-clough! I don't have a TypeScript project using ASR yet, so my peer review here is going to be weak.

---

@ArtskydJ I don't consider that listeners change breaking because the ASR event emitter originally used the node events polyfill supplied by browserify, which doesn't support that second argument to listeners.

eventemitter3 was brought in as a much smaller implementation of that API.

So, there is probably still some code out there that sets setMaxListeners to get rid of the warning that would pop up when you had more than 10 listeners, but I wouldn't expect any code to be relying on the second listeners argument, since the node emitter never had that function.

[whs]

I've updated with the 2 changes @sw-clough have listed.

Thanks for your review

[Vehmloewff]

asr supports returning a promise that resolves to ContentType from state.resolve. See https://github.com/tehshrike/abstract-state-router#returning-values for the docs about this.

Suggested change

	): void | Promise<never>;
	): void | Promise<never> | Promise<ContentType>;