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

Missing Typescript declarations #4697

Open
marcospassos opened this issue Jun 30, 2024 · 5 comments
Open

Missing Typescript declarations #4697

marcospassos opened this issue Jun 30, 2024 · 5 comments
Labels
Feature Request New feature requested in system

Comments

@marcospassos
Copy link

Describe the bug

The NPM module is missing the type declarations.

How to reproduce

  • Install vowpalwabbit/vowpalwabbit
  • Import import promise from '@vowpalwabbit/vowpalwabbit';

Version

0.0.7

OS

Macos

Language

Node

Additional context

No response

@marcospassos marcospassos added the Bug Bug in learning semantics, critical by default label Jun 30, 2024
@JohnLangford
Copy link
Member

We discussed this a bit. My understanding is that this doesn't cause a lack of functionality, but would be desirable from a documentation perspective. Do you want to submit a patch for this?

@JohnLangford JohnLangford added Feature Request New feature requested in system and removed Bug Bug in learning semantics, critical by default labels Aug 15, 2024
@JohnLangford
Copy link
Member

(Closing for now, would be great to have a patch if you want to contribute.)

@marcospassos
Copy link
Author

Hi @JohnLangford, you can't use the JS library with Typescript without typing - the project just won't compile. I'd have to re-declare all the types, which would be error-prone and add more maintenance overhead. The fix is simple: just add the declaration files to the NPM module.

I can provide a PR for this if it's welcome.

@JohnLangford
Copy link
Member

Yes please---a patch would be great.

@JohnLangford JohnLangford reopened this Aug 19, 2024
@marcospassos
Copy link
Author

The current code produces the following typing result:

declare const _default: Promise<unknown>;
export default _default;
//# sourceMappingURL=vw.d.ts.map

This isn't useful at all 😅

I'll work on proper typing and submit a PR, including a declaration file that we can distribute with the package. This will require mapping out the API, so it will take me some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature requested in system
Projects
None yet
Development

No branches or pull requests

2 participants