Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored nodejs bindings to use typescript #546

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

mmende
Copy link
Contributor

@mmende mmende commented May 21, 2021

This PR contains a refactored version of the nodejs bindings using typescript.
This enables autocompletion and static code analysis when including the library in typescript or javascript projects.
JSDocs is a nice start, but when including the library within a typescript project, typescript will complain about a missing declaration file. Furthermore, as code is written in typescript itself, the declaration files doesn't have to be updated manually when working on the library.

I also added prettier and eslint for a consistent code style within the nodejs library and refactored the demo scripts so that the dependencies of the demos must not be included in the library itself.

The PR also makes it a little bit easier to work on the nodejs library by including a script that downloads the shared libraries for development and provides a watch server that reruns a test script once the code changes somehow.

@nshmyrev
Copy link
Collaborator

Hi @mmende

I'm sorry, I'm not very deep in all those JS things, does this change make things incompatible with plain JS? Should we just have 2 parallel version instead?

@mmende
Copy link
Contributor Author

mmende commented May 26, 2021

Hi @nshmyrev

No, this doesn't change anything when using the library with plain JS. The code is transpiled to pure JS and separate type declaration files (.d.ts) in the build step. You can see the output in the dist folder when running yarn run build. A parallel typescript version would also be ok in my opinion even though this would add some redundancy.

@mmende
Copy link
Contributor Author

mmende commented Jul 21, 2021

Added getModelAsync and getSpkModelAsync which allow optionally loading models in a different thread

@alangibson
Copy link

@mmende Have you considered submitting this to DefinitelyTyped? This would be awesome to have, but there doesn't appear to be much appetite to merge it here.

@mmende
Copy link
Contributor Author

mmende commented Dec 8, 2021

@alangibson Yes I thought about it. The problem with separate types however is, that I'd had to keep track of api changes within the vosk nodejs implementation to prevent wrong or outdated type definitions. I also thought about publishing a separate module as @nshmyrev already suggested (maybe something like vosk-ts).

@MartinMuzatko
Copy link

I'd love to see the library to have official type support. If it doesn't get merged, It would be great to have the types seperately or the fork. While the fork is probably easier to maintain

@donyam
Copy link

donyam commented Aug 6, 2022

node_modules\ts-node\src\index.ts:859
return new TSError(diagnosticText, diagnosticCodes, diagnostics);
^
TSError: ⨯ Unable to compile TypeScript:
src/lib.ts:307:7 - error TS2322: Type '{ vosk_model_new: ForeignFunction<Pointer, [string | null]>; vosk_model_free: ForeignFunction<void, [Pointer]>; ... 17 more ...; vosk_gpu_thread_init: ForeignFunction<...>; }' is not assignable to type 'VoskLibrary'.
The types of 'vosk_model_new.async' are incompatible between these types.
Type '(args_0: string | null, args_1: (err: any, value: Pointer) => void) => void' is not assignable to type '(modelPath: string, cb: (err: Error | null, model: Type<Pointer> | null) => void) => void'.
Types of parameters 'args_1' and 'cb' are incompatible.
Types of parameters 'model' and 'value' are incompatible.
Type 'Pointer' is missing the following properties from type 'Type<Pointer>': size, indirection, get

307 const libvosk: VoskLibrary = ffi.Library(soname, {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants