search

proj4js / mgrs

Utility for converting between WGS84 lat/lng and MGRS coordinates
MIT License
105 stars 45 forks source link

Add TS typings + build improvements #54

Closed thw0rted closed 5 years ago

thw0rted commented 5 years ago

Hi @DanielJDufour , I came to add TS typings and wound up fixing a few build issues as well. I tried to make each fix as its own commit so you can examine them separately. I can also make separate PRs if you'd like, but the overall scope is still pretty small so I thought it might be OK to do as one.

I've tested the reconfigured build and it works in my consuming applications, but I didn't add a "smoke test" or anything automated that uses the build artifact.

DanielJDufour commented 5 years ago

Awesome. I'll take a look!

thw0rted commented 5 years ago

Give me a couple minutes, I didn't notice that Travis failed. I was futzing with the rollup config for terser and didn't re-run tests afterwards. (The good news is that apparently the tests do use the build artifact, which I should have noticed. Sorry!)

DanielJDufour commented 5 years ago

No worries :-)

thw0rted commented 5 years ago

OK, the commit I'm pushing now uses resolve('../') which checks the main field in package.json to determine where to find the entry point. I was going to try to add a second test set, or at least an alternate mode for testing, that consumes the ESM package, but apparently Mocha is still working on built-in ESM / MJS support, so I'll have to save that for another day.

DanielJDufour commented 5 years ago

I made just a couple comments. Other than that, looks good to go!

thw0rted commented 5 years ago

Hopefully that clears things up, if you want to make any changes (or just revert any of the "fixes" I put in) please feel free.

DanielJDufour commented 5 years ago

Awesome! Thanks for the quick feedback :-)

thw0rted commented 5 years ago

FYI that change I made is from the GH in-browser text editor. Since you're a maintainer here and I set the PR to "allow edits", you should also have permissions to do that. (I don't mind doing it, I just wanted to make sure you were aware.)