HermannBjorgvin / Kennitala

Icelandic national ID (kennitölur) utilities for servers and clients.
MIT License
36 stars 10 forks source link

Adding typescript declaration file #11

Closed dabbeg closed 4 years ago

dabbeg commented 4 years ago

Resolves: #10

I modified the declaration file slightly from the original post https://github.com/HermannBjorgvin/Kennitala/issues/10#issuecomment-698536408. I fixed a few typos and modified the kennitala parameter to accept string and number

HermannBjorgvin commented 4 years ago

Have you tested to see if the type declaration file works? I'm not in the TypeScript club but I'm curious about one thing.

What I'm concerned about is that the distribution files that I point to in the main property of package.json is obfuscated code with the variable names changed. How does the type declaration file map those obfuscated variable names to the type declaration file?

https://github.com/HermannBjorgvin/Kennitala/blob/91ad66d4a0fb4ea87241b45c6c0bf997cd9ac477/package.json#L5

dabbeg commented 4 years ago

Yes I tested it and it works fine. :muscle:

The uglify step changes all the variables names that are not exposed in any way. So if you checkout the min file, you should see that all the function names and .info properties have not changed and remain the same as they are in the declaration file. Typescript deals only with the interface that the user interacts with.

Here are some screenshots :smiley: : 2020-09-29-113206_852x199_scrot 2020-09-29-113237_935x368_scrot 2020-09-29-113628_411x83_scrot

HermannBjorgvin commented 4 years ago

That's good to know. The changes look good. I'll try to merge this tonight or tomorrow and bump the version to 1.2.4 since we have no breaking changes to worry about. I'm gonna go over the readme and add some stuff about typescript support.

HermannBjorgvin commented 4 years ago

Merging this onto v1.2.4 to do some README.md changes before pushing to master and doing all the NPM publish stuff

HermannBjorgvin commented 4 years ago

@dabbeg is it recommended to include the types file in package.json?

"types": "./dist/kennitala.min.d.ts"

See the following: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package

dabbeg commented 4 years ago

Hmm, I am not sure exactly for what purpose it serves as this worked for me without it. But it looks like it is recommended so we should probably add it.

Can you take care of that?

HermannBjorgvin commented 4 years ago

I'll include it in the version bump no problem