fix(jco): invalid TypeScript types for @bytecodealliance/jco API#1506
Merged
Conversation
Contributor
Author
|
Hm, just realizing those functions being referenced from '../obj/wasm-tools.js` don't actually exist. |
Contributor
Author
I made the parameter types in the JSDoc comments for |
youngspe
commented
May 19, 2026
vados-cosmonic
previously approved these changes
May 19, 2026
Collaborator
vados-cosmonic
left a comment
There was a problem hiding this comment.
LGTM 🚀
Thanks @youngspe for adding this -- let's get the tests running and merge this.
Collaborator
|
One thing I'll do is squash and rename your commits -- since our conventional commits check is failing -- and a fmt. |
5a9c947 to
496fa78
Compare
496fa78 to
af3347c
Compare
af3347c to
6817a92
Compare
vados-cosmonic
approved these changes
May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently the TypeScript types for
@bytecodealliance/jcoare broken. There are threeissues:obj/wasm-tools.jsbut innpm publishthe type declarations are generated beforeobj/is populated. This causes types to be emitted asanyinstead of their actual types. For example:Expected:
Actual:
src/api.jsreference nonexistent exports ofobj/wasm-tools.js. For example,Parameters<import('../obj/wasm-tools.js').metadataAdd>[0]effectively resolves toanybecause there is no type calledimport('./obj/wasm-tools.js').metadataAdd. This was likely intended to beParameters<typeof import('../obj/wasm-tools.js').tools.metadataAdd>[0], which resolves toUint8Array.src/api.jshas relative exports from./cmd/opt.jsetc. but the types for thecmddirectory are kept insrcwhile the rest of the types are intypes. This meanstypes/api.d.tstries to referencetypes/cmd/opt.d.tsbut there's no file there so TypeScript can't resolve it. This makes theopt,transpile, andtypesexports completely untyped.This change does the following:
xtask build ..so thattscis run after the components referenced by the API are transpiled.packages/jco/tsconfig.json:includeto just"src"because TypeScript already recursively searches included directoriesexclude:node_modulesortestdirectories undersrc/to exclude from compilation, andexcludewon't have an effect on files not referenced byinclude.src/cmd/**means type declarations for files insrc/cmd/are generated undertypes/cmd/so relative imports fromtypes/work as expected.*.d.tsfiles don't need to be excluded becausetscdoesn't generate any output files for them."rootDir": "src". This is already implied, but ts5.9 gives a deprecation warning because it's required in ts6.0src/cmd/and regeneratetypes/.packages/jco/src/api.js:// @ts-checkto enable type checking for just that file to ensure that the parameters are consistent with the functions fromwasm-tools.jsIn case it looks like it from this description, this PR is not AI-generated, I just like over-explaining and making nested lists