-
Notifications
You must be signed in to change notification settings - Fork 261
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
Updated TypeScript definitions #102
Updated TypeScript definitions #102
Conversation
this uses the tsd-jsdoc npm package https://www.npmjs.com/package/tsd-jsdoc it gets a few errors and no idea how close the types are. Maybe someone more familar with typescript can take a look? checked in the generated types here You can run it with npm install ./node_modules/.bin/grunt jsdoc:ts
/** comment */ was being confused as a docstring and resulting in TS declaration pollution
use npm run buildts to re-run the ts generator and apply the script
Fixes this typo in docs and in the "mungets" task: WebGLContextCreationAttirbutes => WebGLContextCreationAttributes
All fields are optional but lacked the [default brackets] that would let grunt know about this when outputting the TypeScript definitions
These functions were documented as accepting a "subdivisionsThick" argument after "subdivisionsDown" but before "startOffset", though the function does not in fact accept such an argument. Error was revealed by WIP TypeScript test.
"npm run test" will now run both the pre-existing "js" tests as well as the new "ts" test. Use "npm run testjs" to run only the earlier tests. Borrows a handful of lines from code in the examples/ directory. It doesn't need to do anything interesting, it just needs to successfully compile. The TS test is not very thorough at this point, but it's a very solid start imo.
This is also probably the source of the build failure in commit 0e66391
Remove "types.d.ts" Add "twgl-full.d.ts" and "twgl.d.ts"
content = content.replace(/\bTypedArray\b/g, 'ArrayBufferView'); | ||
// What docs call an "augmentedTypedArray" is technically an "ArrayBufferView" | ||
// albeit with a patched-in "push" method. | ||
content = content.replace(/\baugmentedTypedArray\b/g, 'ArrayBufferView'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO (though outside the scope of this PR): Describe this type more accurately. I think this would work but I'm not certain.
export interface AugmentedTypedArray extends ArrayBufferView {
push: (..args: any[]): void;
}
// Docs use "constructor"; TS expects something more like "Function" | ||
content = content.replace(/: constructor/g, ': Function'); | ||
// Docs use "ArrayBufferViewType" to describe a TypedArray constructor | ||
content = content.replace(/\bArrayBufferViewType\b/g, 'Function'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO (though outside the scope of this PR): I think this input constructor type can be described more accurately. I think this typing would work but I'm not certain.
export function getGLTypeForTypedArrayType(typedArrayType: ((...args: any[]) => ArrayBufferView)): number;
My gut says I'd prefer it not in the global scope. see #103 |
Previously in the global scope (e.g. twgl.Vec3) and now in module scope (e.g. twgl.v3.Vec3)
@greggman Ok, that makes sense. I added a new commit which puts those type declarations in the module scopes. |
because I can
Closing because these commits have been incorporated into the v4.8.0 release: |
TypeScript definitions can now be built ("npm run buildts") and tested ("npm run testts").
Builds on the work in #97 and #98
@greggman Do you have strong feelings about the Vec3 and Mat4 types being in the global scope (
twgl.Vec3
/twgl.Mat4
) vs. in module scopes (twgl.v3.Vec3
/twgl.m4.Mat4
)? The latter module scope option would more closely line up with exactly what the docs say, but I used the former global scope option because I thought that it would be most dev's preference. (Certainly my own.) I'd be fine changing it to the other if you'd rather they be in module scope.