feat(TS): expose TS interfaces, add type tests

[BendingBender]

This PR contains:

• bugfix
• feature
• refactor
• tests
• documentation
• metadata

Breaking Changes?

• yes
• no

If yes, please describe the breakage.

Please Describe Your Changes

• exposes module TypeScript interfaces, especially the Options interface
• adds doc comments to TS types, this improves inline documentation for JS and TS users alike
• fixes incorrectly typed skipComments option
• adds tests for the whole exposed TS interface, adds the tsd helper to run the tests automatically along with unit tests
• refactors types in readme to use TS type notation which is the most common type notation in the JS ecosystem

[shellscape]

Thanks for the PR! 🍺

refactors types in readme to use TS type notation which is the most common type notation in the JS ecosystem

I'd say this is very subjective, and while I appreciate the effort that went into the changes you made, this module is not a TypeScript project and the type grammar as it was in the README is maintainer preference. If there's a spec that governs this within project documentation that I'm unaware of, please do link it and I'll reconsider. TypeScript does not yet rule the world 😄

I would also prefer the typedefs moved out of the project and into https://github.com/DefinitelyTyped/DefinitelyTyped where they can be properly tested, maintained, and supported, separately.

[BendingBender]

I'd say this is very subjective, and while I appreciate the effort that went into the changes you made, this module is not a TypeScript project and the type grammar as it was in the README is maintainer preference.

I was not aware of these preferences, I thought that it will actually make maintenance easier. I mean the style I've replaced wasn't even used consistently so I thought that this will be an improvement. Will revert.

I would also prefer the typedefs moved out of the project and into DefinitelyTyped/DefinitelyTyped where they can be properly tested, maintained, and supported, separately.

Actually, DefinitelyTyped recommends to post type definitions directly to projects and only post them there as a last resort. There is no one at DT who does this "properly tested, maintained, and supported". This is all done by volunteers, there are no active maintainers. It's also a much better developer experience if one doesn't have to install and maintain extra @types dependencies. Another problem is that there is no direct mapping of package versions at DT to the corresponding package versions of a project so one never actually knows whether the types will work with a package at all if the types are not up to date.

The chances are much higher that if someone changes the interface here he will also update the types. This is much less likely if the types are hosted at DT.

All in all I wouldn't recommend moving the types there.

[shellscape]

Generally speaking for projects that I maintain, when the typedefs are simple I keep them local. When they get complex to the point where I have to learn TypeScript to maintain the definitions, I request that they be externally hosted. For folks who are fans of TypeScript and/or work with it often, this is a non-issue. For others, it's an additional maintenance burden.

I'm all for making things more convenient for other developers, but this is not a TypeScript project (at least, not at present). And what precious little time I have for open source these days doesn't allow for maintaining definitions for a language variant that I have no interest in. Honestly - I don't even know what the functional difference between a namespace and module are in that context, so I can't even comment on whether or not that's a good change. At present I'm the only active maintainer for the project and it would be irresponsible for me to push a change out to users I didn't comprehend.

If this is something you're particularly passionate about, it might be good to petition @mafintosh to add you as a maintainer so you can convert it to TypeScript and take over maintenance. I wouldn't mind that in the least.

[BendingBender]

If you don't want to review future changes in TS types I will happily do this for you if you CC me.

To explain my changes: The module declaration was superfluous and the namespace I've introduced effectively allows exporting the Options interface which was previously internal to the module and not usable outside of the local definitions.

I've mostly followed the DT styleguide and added tests for the whole module interface to make sure that isn't broken by me and it won't be broken by another refactoring.

[BendingBender]

So do I understand you correctly, you would rather prefer moving the whole definition to DT instead of accepting my changes?

Then we'll have to provide a migration path for users of this module. We will ultimately have to remove the types here and break most TS users.

I can do the submission to DT. Any ideas on how you would handle the breakage?

[BendingBender]

@mafintosh Would you mind adding me as an additional maintainer to this project so that I can help managing the TS defintion?

[mafintosh]

@BendingBender added you, whats your npm username?

[BendingBender]

It's bendingbender.

[BendingBender]

@shellscape Are you OK with me merging this now? @mafintosh added me as a maintainer, I'll take care of the TS definitions if you don't mind.

[shellscape]

@BendingBender that sounds good to me. welcome aboard!

[sindresorhus]

@shellscape Can you do a new release?

[shellscape]

Sure thing. It'll probably be tomorrow morning (ET). I'm away until late tonight.

[shellscape]

@BendingBender is this a major or minor release in your opinion?

[BendingBender]

Should be minor IMO.

The only problematic thing is the fixed type for the skipComments option. It was incorrectly typed, I corrected it. It is debatable whether this is a breaking change. It will break users who were relying on the incorrect type but their programs should already have a bug there.

All the other changes are backwards-compatible.