Feature typescript typegen

[zachasme]

I needed typescript definitions for these bindings and noticed you generate your README off of jsdoc.

I've managed to generate the definitions using tsd-jsdoc requiring minimal change to h3core.js.

This PR adds the required jsdoc changes, but also a script to bundle the types on prepublish.

I'm not sure how you feel about distributing these definitions yourself. If you'd rather not, I'll simply remove that part. But I do hope you'll allow the jsdoc changes. 😃

[nrabinowitz]

This is great! Thanks for the contribution - I've been wanting to add typedefs for TS and Flow for a bit, but hadn't considered this approach.

The build is currently failing because the generated README file has changed due to JSDoc changes (specifically the module name h3-js). The easy fix is to run yarn build-docs locally and check in the results. But this is unfortunately a place where the JSDoc and TS requirements seem to diverge - h3-js is the package name, but it can't be the import name, so it's weird to have h3-js.h3ToGeo in the docs. I can't see any way to fix this in tsd-jsdoc itself.

Assuming that we'd like to gen typedefs without changing the docs, I think either the code might need pre-processing, or the README or typedef might need post-processing... I think the easiest might be changing the command for docs generation to pipe through sed:

"build-docs": "jsdoc2md --global-index-format grouped --partial doc-files/scope.hbs --helper ./doc-files/insert-version.js --separators --template doc-files/README.tmpl.md lib/h3core.js | sed 's/h3-js\\./h3./g' > README.md"

Open to better options if you have one - this only fixes worst doc issue, but it's probably sufficient.

[nrabinowitz]

Sorry, just gave you a merge conflict as well - you'll need to rebase to master.

I'm not sure how you feel about distributing these definitions yourself.

I'm not sure what the standard approach here is, but I'm happy to include the typedefs in the dist if that's appropriate for TS users. Now I'm wondering whether we can do this for Flow too.

[zachasme]

Hm, I hadn't considered the generated docs. I can see the H3Index type also adds another layer to the table-of-contents which seems unnecessary.

I like your idea of pre- or post-processing. I'll try finding a solution that doesn't change the docs.

The think the standard approaches for non-typescript projects is either bundling it yourself or leaving it to userland to provide them through the DefinatelyTyped project. The first approach is preferable IMO.

Side note: Have you considered distributing the library simply as 'h3' on npm? Putting 'js' or 'node' in the package name is discouraged.

[nrabinowitz]

Side note: Have you considered distributing the library simply as 'h3' on npm? Putting 'js' or 'node' in the package name is discouraged.

The name was taken. It's now deprecated, but would probably require some work (and invoke some potential confusion) to try to take it over.

I also think the h3-js case is a little non-standard as well, because it's a binding to a library in another language. All of the official bindings share the same naming structure - h3-py, h3-java, etc - so calling h3-js just h3 might actually be confusing, especially since it would make it harder to talk about the relationship to the core C lib. Plus there's actually a separate Node-only h3-node project (offering true C bindings), so it's useful to differentiate.

[nrabinowitz]

Hm, I hadn't considered the generated docs. I can see the H3Index type also adds another layer to the table-of-contents which seems unnecessary.

One other idea here is that you could concatenate either source or TS code during the generation process to shim in missing defs.

[coveralls]

Pull Request Test Coverage Report for Build 119

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 117:	0.0%
Covered Lines:	351
Relevant Lines:	351

---

💛 - Coveralls

[zachasme]

I was not aware of the separate N-API module. Seems like a natural way to go then 👍

I've rebased and moved the module renaming to a simple pipe through sed as per your suggestion.

I also added a @static tag to the H3Index typedef, which avoids the TOC nesting.

[nrabinowitz]

Looks great. Thanks for the contribution!

[nrabinowitz]

TIL :). This works for me.

[nrabinowitz]

👍