From 26f68744b5d69d73ddb2fcc2d1d21ec25a427059 Mon Sep 17 00:00:00 2001 From: Matt Tracy Date: Tue, 29 Sep 2015 14:05:53 -0400 Subject: [PATCH 1/2] Improve Typescript definition for MithrilRoutes The previous definition of MithrilRoutes was generic over , with the MithrilRoutes containing a collection of MithrilComponent. However, this presents problems with TypeScript's type inference in many common situations. For example, consider this usage of m.route(): ``` m.route(document.body, "/a", { "/a": ComponentA, "/b": ComponentB, }) ``` Both `ComponentA` and `ComponentB` implement `MithrilComponent`, with each component having a different concrete controller type (`ControllerA` and `ControllerB`). However, unless ControllerA's type is a subset of ControllerB's type (or vice-versa), TypeScript will be unable to infer the type of the MitrilRoutes returned by m.route(). To get this to compile, a third type which *is* a subset of both ControllerA and ControllerB would need to be explicity provided: ``` // Both ControllerA and ControllerB implement MithrilController m.route(document.body, "/a", { "/a": ComponentA, "/b": ComponentB, }) ``` This commit makes MithrilRoute non-generic, and instead of containing MithrilComponent, it contains MithrilComponent, which will match all valid MithrilComponents. The motivation for this change is twofold: 1. In the case where m.route() does not compile due to type-inference failure, the resulting syntax error from Typescript is very unhelpful because m.route() has a number of overloads. Leaving this as is will result in confusion for downstream users. 2. An assumption that vanishingly little utility is provided by defining `MithrilRoutes` over a type more specific than MithrilController. At most, it could be used to assert that all of your routed Components meet a more specialized interface, but that same type check could be accomplished without having to augment MithrilRoutes with that information. --- mithril.d.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mithril.d.ts b/mithril.d.ts index a5c24a6e..2952d92a 100644 --- a/mithril.d.ts +++ b/mithril.d.ts @@ -431,10 +431,10 @@ declare module _mithril { * @param defaultRoute The route to start with. * @param routes A key-value mapping of pathname to controller. */ - ( + ( rootElement: Element, defaultRoute: string, - routes: MithrilRoutes + routes: MithrilRoutes ): void; /** @@ -447,7 +447,7 @@ declare module _mithril { * m("a[href='/dashboard/alicesmith']", {config: m.route}); * ``` */ - ( + ( element: Element, isInitialized: boolean, context?: MithrilContext, @@ -876,12 +876,12 @@ declare module _mithril { /** * This represents a key-value mapping linking routes to components. */ - interface MithrilRoutes { + interface MithrilRoutes { /** * The key represents the route. The value represents the corresponding * component. */ - [key: string]: MithrilComponent; + [key: string]: MithrilComponent; } /** From da6d525a11bf4e21bb69da8f1561fcbac2a72400 Mon Sep 17 00:00:00 2001 From: Matt Tracy Date: Tue, 29 Sep 2015 14:44:34 -0400 Subject: [PATCH 2/2] Typescript: Make MithrilVirtualElement non-generic `MithrilVirtualElement` was recently converted to be generic over ``. The element's `children` may then contain other `MithrilVirtualElement` or `MithrilComponent` elements. Unfortunately, in common usage of 'm()' this can cause problems with Mithril's type inference system. If a type is not explicitly provided to `m()`, the "T" of the returned MithrilVirtualElement will be inferred from its children. The candidates for T will be the concrete types of the child MithrilComponent; however, if no candidate type is a subset of *every* candidate type, then TypeScript will be unable to infer a valid type, and the call to `m()` will not compile. To improve this behavior, this commit makes MithrilVirtualElement non-generic; it's child collection can now contain MithrilComponent, which matches all valid MithrilComponent. This the same underlying issue as the previous commit, which made MithrilRoutes non-generic. The motivation for fixing this is threefold: 1. Because `m()` has so many overloads, in the case where a call to `m()` will not compile due to the above issue, the syntax error provided by TypeScript is not helpful. 2. In many cases, users will be calling `m()` in their code simply to get it to compile, which provides no additional utility over a non-generic type and adds significant boilerplate. 3. Explicitly specifying subtypes to `m()` calls provides little utility; at best, it can check that contained components implement some common interface. However, type assertions like that can be provided in other places without having to attach the information to the MithrilVirtualElement. --- mithril.d.ts | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/mithril.d.ts b/mithril.d.ts index 2952d92a..c7d2c6b6 100644 --- a/mithril.d.ts +++ b/mithril.d.ts @@ -20,13 +20,13 @@ declare module _mithril { * @see m.mount * @see m.component */ - ( + ( selector: string, attributes: MithrilAttributes, ...children: Array | - MithrilComponent> - ): MithrilVirtualElement; + MithrilVirtualElement | + MithrilComponent> + ): MithrilVirtualElement; /** * Creates a virtual element for use with m.render, m.mount, etc. @@ -40,12 +40,12 @@ declare module _mithril { * @see m.mount * @see m.component */ - ( + ( selector: string, ...children: Array | - MithrilComponent> - ): MithrilVirtualElement; + MithrilVirtualElement | + MithrilComponent> + ): MithrilVirtualElement; /** * Initializes a component for use with m.render, m.mount, etc. @@ -364,9 +364,9 @@ declare module _mithril { * @param forceRecreation If true, overwrite the entire tree without * diffing against it. */ - render( + render( rootElement: Element, - children: MithrilVirtualElement|MithrilVirtualElement[], + children: MithrilVirtualElement|MithrilVirtualElement[], forceRecreation?: boolean ): void; @@ -451,7 +451,7 @@ declare module _mithril { element: Element, isInitialized: boolean, context?: MithrilContext, - vdom?: MithrilVirtualElement + vdom?: MithrilVirtualElement ): void; /** @@ -615,7 +615,7 @@ declare module _mithril { * * @see m */ - interface MithrilVirtualElement { + interface MithrilVirtualElement { /** * A key to optionally associate with this element. */ @@ -634,7 +634,7 @@ declare module _mithril { /** * The children of this element. */ - children?: Array|MithrilComponent>; + children?: Array>; } /** @@ -686,11 +686,11 @@ declare module _mithril { * @param context The associated context for this element. * @param vdom The associated virtual element. */ - ( + ( element: Element, isInitialized: boolean, context: MithrilContext, - vdom: MithrilVirtualElement + vdom: MithrilVirtualElement ): void; } @@ -758,7 +758,7 @@ declare module _mithril { /** * Creates a view out of virtual elements. */ - (ctrl: T): MithrilVirtualElement; + (ctrl: T): MithrilVirtualElement; } /** @@ -781,7 +781,7 @@ declare module _mithril { * * @see m.component */ - view(ctrl: T): MithrilVirtualElement; + view(ctrl: T): MithrilVirtualElement; } /** @@ -805,7 +805,7 @@ declare module _mithril { * * @see m.component */ - view(ctrl: TController, arg1: T1, arg2: T2, arg3: T3, arg4: T4, ...args:any[]): MithrilVirtualElement; + view(ctrl: TController, arg1: T1, arg2: T2, arg3: T3, arg4: T4, ...args:any[]): MithrilVirtualElement; } /**