Fix confusing type errors by reversing order of overloaded h() definitions

[garybernhardt]

For overloaded functions, TypeScript reports errors based on the last of the overloaded definitions. For h's overloaded definitions, the string version is currently last. When type checking of an h call fails (for example, due to a missing prop, or due to an incorrect prop type), TypeScript simply says "Argument of type 'typeof OurComponent' is not assignable to parameter of type 'string'." This patch switches the order of the h definitions, so that the h<P> version comes last. This makes TypeScript report a more specific error that will include missing props, etc.

Before this change:

$ tsc
src/mycomponent.ts:5:14 - error TS2345: Argument of type 'typeof MyComponent2' is not assignable to parameter of type 'string'.

5     return h(MyComponent2, {})

After this change:

$ tsc
src/mycomponent.ts:5:28 - error TS2345: Argument of type '{}' is not assignable to parameter of type 'Attributes & Props'.
  Type '{}' is not assignable to type 'Props'.
    Property 'foo' is missing in type '{}'.

5     return h(MyComponent2, {})

See my working example repo. Switch the order of the h definitions in src/preact-8.2.9.min.d.ts in that repo and you'll see that the error goes from the first (confusing) error above to the second (very clear) error.

[coveralls]

Coverage remained the same at 100.0% when pulling 0aece17 on garybernhardt:reverse_h_type_definition_order into b243a5e on developit:master.

[developit]

Seems good to me.

[marvinhagemeister]

I'd love to verify this first. Usually the last definition in TS represents the actual implementation in an overloaded function. A third combined definition is missing here. Switching them around would only disable one of them, but I'll have to check to be sure.

[marvinhagemeister]

Just tried out the changes in the PR but they fail for this test case:

import { h } from "preact";

class Foo extends Component {
	render() {
		return <div>Hello</div>
	}
}

function Bar() {
	return <div>World</div>
} 

h("div", {}, h(Foo, null), h(Bar, null));

with the following error message:

[ts] Argument of type 'typeof Foo' is not assignable to parameter of type 'string'.

[garybernhardt]

I can't reproduce that failure, but I also don't use JSX and have since given up on TypeScript.

[gpoitch]

This PR fixes it for me

[garybernhardt]

@marvinhagemeister I misunderstood your comment before. I think you mean that even with this patch, the uninformative error message is sometimes shown? Unfortunately I don't know how to install my own preact fork as a dev dependency (it's not requireable when I try), so I can't test whether this PR at least improves the frequency of the "good" error message showing up.

This issue is far and away my biggest annoyance when using preact with typescript, so it would be really good to have it fixed. Unfortunately, changes beyond the simple reordering in this PR are well beyond my meager understanding of TS at this point. Have you given any thought to adding that third combined definition that you mentioned?

[gpoitch]

#1246 adds a test for this and another type fix for non-jsx preact

[garybernhardt]

Ahh, that sounds great. There's also a TypeScript issue to fix the TS error message itself (microsoft/TypeScript#27249), but who knows when that will land.

[marvinhagemeister]

Just looked at this issue again and I can confirm that it indeed does fix the issue. Not sure why I was getting a failure when previously reviewing the changes. On top of that I stand corrected with the number of overloads declared inside a declaration file. I was under the impression that they had to be declared the same way in a .d.ts file as in a .ts file, but I was wrong. The actual implementation signature must be skipped for d.ts files.

@garybernhardt I'm really thankful that you brought this issue up 👍 Issues like these are not easy to spot :) Congratulations for your first contribution to preact 👍 🎉

[garybernhardt]

Thanks for merging!