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

Provide Typescript type declarations for jco API #253

Closed
landonxjames opened this issue Nov 7, 2023 · 5 comments · Fixed by #256
Closed

Provide Typescript type declarations for jco API #253

landonxjames opened this issue Nov 7, 2023 · 5 comments · Fixed by #256
Labels
enhancement New feature or request TypeScript TypeScript output generation

Comments

@landonxjames
Copy link
Contributor

landonxjames commented Nov 7, 2023

I am attempting to use jco as a library from a Typescript project. The following import throws an error:

import { transpile } from "@bytecodealliance/jco";

Throws:

Could not find a declaration file for module '@bytecodealliance/jco'. '/Volumes/workplace/project/node_modules/@bytecodealliance/jco/src/api.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/bytecodealliance__jco` if it exists or add a new declaration (.d.ts) file containing `declare module '@bytecodealliance/jco';`ts(7016)

This appears to be because there are no .d.ts files in node_modules/@bytecodealliance/jco, so ts doesn't recognize the package. I can add my own declaration file to get unblocked, but this seems like a thing that will impact multiple users going forward.

From some quick research it seems like this could be handled in a few ways:

  • Change jco to a typescript project, run tsc as part of the build to generate the .d.ts bindings
  • Manually write ts bindings and distribute them with the package
  • Manually write the bindings and contribute them to DefinitelyTyped

The first option looks like the best, but would be fairly disruptive. The other options might be quicker.

@landonxjames
Copy link
Contributor Author

Happy to contribute to some work here, but would like feedback from project maintainers on what direction is preferable before I dive too far in.

@landonxjames landonxjames changed the title Provide Typescript types for jco API Provide Typescript type declarations for jco API Nov 7, 2023
@yoshuawuyts yoshuawuyts added enhancement New feature or request TypeScript TypeScript output generation labels Nov 9, 2023
@guybedford
Copy link
Collaborator

Thanks for bringing this up, we should definitely resolve it!

The file in src/api.js directly reexports from files that are all themselves typed, so it should be possible to achieve this with just an api.d.ts or jsdoc comments that reexport from the same interfaces.

So it should be a relatively PR to fix, and we shouldn't need to maintain much more than just those reexport declarations.

If you're able to look into this further that would be a huge help.

@landonxjames
Copy link
Contributor Author

landonxjames commented Nov 13, 2023

Almost done with this, but I'm encountering an error that I've encountered in my own projects as well. The file obj/wasm-tools.js referenced by the JSDoc types contains the following:

export { tools, tools as 'local:wasm-tools/tools' }

This is valid javascript, but (seemingly) not valid typescript. Ts accepts as fooBar, but not as "fooBar". This is probably a ts bug since it should mirror the js implementation.

In my own projects I've been handling this by seding in a replacement before running tsc, but that feels a but hacky for the project that generates those export statements. It should work here though as nothing appears to be using that alias.

Created a bug in the ts repo to track this: microsoft/TypeScript#56375

@landonxjames
Copy link
Contributor Author

Ahh turns out this is already a known ts bug, but the fix for it seems to have stalled out: microsoft/TypeScript#40594

@guybedford
Copy link
Collaborator

Yes, for TypeScript definitions, we just have to omit these export names for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request TypeScript TypeScript output generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants