Fix TypeScript types

[wingrunr21]

This resolves #1108

• Adds types property to package.json (see Publishing docs)
• Revamps the typings.d.ts file to more accurately reflect both the internal and external API. Specifically, the Options interface should have every property marked optional (for external consumption) but most of those properties are assumed to be not null/undefined throughout index.js. I thus introduced an InternalOptions interface which matches this requirement.
• Moved the Hooks typedef into the typings.d.ts file vs defining it inline.
• Moved HtmlTagObject inside the HtmlWebpackPlugin namespace.

I did not fix compatibility with noImplicitAny in this PR. As you are shipping an annotated JS file, I believe TS users downstream with both noImplicitAny and allowJs enabled will get a bunch of compiler warnings.

cc @SimonSchick

[jantimon]

I really like what you did here!

Unfortunately sorting the options made it very hard to review this pull request.
Could you please add some changes like described above?

[wingrunr21]

Sorry about the Options sorting thing. Should have dropped that in an independent commit. I'll push some changes!

[jantimon]

Just realized that the Require<Omit does not work. All properties are still optional.

[wingrunr21]

Hmm, I tested it locally. Omit is for sure working. Let me look at it.

[wingrunr21]

Ok, scratch that. Looks like Omit is the one causing the problem. That's odd as that's the official definition of the type (see here). I'll keep looking.

[wingrunr21]

Ok, so apparently I didn't need to redefine Required as in TS 2.8+ it is part of lib.d.ts.

The other part is apparently you can remove the optionality aspect of a type but you can't remove its ability to be set to undefined (see here).

So, this approach may not actually solve the use case I was trying to solve. As it's still internal it may not be that big of a deal but...

[jantimon]

Well right now the types were used to guarantee the quality and to reduce bugs..
Why would that be not a big deal?

[wingrunr21]

Because they weren't accurate to begin with. If you're that worried about it feel free to revert this PR. Your TS support for downstream users is completely broken though. How are they supposed to guarantee quality and reduce bugs?

[jantimon]

They were very accurate for all internal flows and there was @types/html-webpack-plugin for downstream users.

If an internal function relies on values which are not present it might cause bugs in edge case situations

What was not accurate?

[wingrunr21]

I'm not going to debate this, sorry. I think #1108 + the explanations in this PR cover the reasons.

So, looks like I spoke too soon. I poked at this some more and the problem is actually the wildcard [option: string]: any line at the bottom of the Options interface. If you comment that out then the Required<Omit stuff works the way it is supposed to.

You can see a simple example here on the TS playground. Comment out line 4 and you can see the errors change accordingly.

[jantimon]

I guess the cleanest way would be to write two interfaces or to start with the ProcessedOption and use Partial again

[jantimon]

Before we relied on the webpack source code - now we rely on @types/webpack

[wingrunr21]

Before we relied on the webpack source code - now we rely on @types/webpack

No it doesn't. See https://github.com/jantimon/html-webpack-plugin/blob/master/index.js#L7-L8

[jantimon]

But why did we have to add @types/webpack ?

[wingrunr21]

https://github.com/jantimon/html-webpack-plugin/blob/master/typings.d.ts#L1

[jantimon]

Thanks good hint :)

I’ll try to refactor those changes on a new or tomorrow

[jantimon]

@wingrunr21 I fixed the remaining open points - would love to get your feedback:

f4cb241