Add typescript type definitions#7
Add typescript type definitions#7clems4ever wants to merge 1 commit intoashtuchkin:masterfrom clems4ever:type-definitions
Conversation
|
For you information, I've also written the type definitions for the u2f-api npm package that I use in combination with u2f package in my project (https://www.npmjs.com/package/authelia). |
|
Hello @ashtuchkin , did you have time to check the PR? |
|
Out of interest: how does the testing of the typings work? You just have included the |
ashtuchkin
left a comment
ashtuchkin
left a comment
There was a problem hiding this comment.
Overall looks great, thank you for your contribution! Added a couple comments inline.
index.d.ts
Outdated
| successful: boolean; | ||
| publicKey: string; | ||
| keyHandle: string; | ||
| certificate: string; |
There was a problem hiding this comment.
nit: certificate is a Buffer.
index.d.ts
Outdated
| } | ||
|
|
||
| export interface RegistrationResult { | ||
| successful: boolean; |
index.d.ts
Outdated
| certificate: string; | ||
| } | ||
|
|
||
|
|
| "devDependencies": { | ||
| "mocha": "2" | ||
| "@types/node": "^7.0.22", | ||
| "mocha": "2", |
There was a problem hiding this comment.
removed @types/node and added ts-node instead.
package.json
Outdated
| }, | ||
| "scripts": { | ||
| "build": "./node_modules/.bin/tsc -p ." | ||
| }, |
There was a problem hiding this comment.
could you remove this clause? it's not actually a build step, so might be confusing to others.
| @@ -0,0 +1,98 @@ | |||
|
|
|||
There was a problem hiding this comment.
Thank you for writing tests! I really appreciate it.
This file is not using mocha, though, so it can be confusing. Can you try to make it a real test? This article can help: https://journal.artfuldev.com/unit-testing-node-applications-with-typescript-using-mocha-and-chai-384ef05f32b2
Or, if you want to just keep it as an example of how typescript would work, then can you rename this file to typescript-example.ts, so it doesn't look like a test.
There was a problem hiding this comment.
done. One can run the tests with mocha -r ts-node/register test/ts-test.ts.
tsconfig.json
Outdated
| @@ -0,0 +1,21 @@ | |||
| { | |||
There was a problem hiding this comment.
This project is not built in typescript, so this file could be confusing, could you remove it if possible? I think we can use node-ts module to avoid compilation step altogether.
There was a problem hiding this comment.
File removed in favor of node-ts.
|
Sorry for such a long delay! |
Also add some unit tests that can be run with: `mocha -r ts-node/register test/ts-test.ts`
|
Everything should be fixed now. Sorry for the long delay as well :). |
3 participants
Add type definitions and a test file in typescript to test the compilation.
npm run buildcompiles the typescript test file in directory build/ so that you can run it and check everything is working fine.