-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Add sized array types #11
Conversation
Pull Request Test Coverage Report for Build 10115381086Details
💛 - Coveralls |
modules/types/src/array-types.ts
Outdated
number | ||
]; | ||
|
||
/** |
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.
Why are all these declarations being added?
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.
Why are all these declarations being added?
These came from gl-matrix. I am still evaluating various approaches to typing these things. But removed them for now.
TypedArray, | ||
TypedArrayConstructor, | ||
NumericArray, | ||
NumberArray, |
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.
Could we remove NumberArray
? I don't see why we need an alias for NumericArray
and it feels off to have NumberArray
support typed arrays while NumberArrayX
explicitly does not
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.
Could we remove NumberArray?
Unfortunately it is used a lot, so that would best be done in a major release.
I don't see why we need an alias for NumericArray
No, we don't need two types. I was actually trying to phase out NumericArray
, but take your point on other NumberArrayX
not supporting typed arrays, so perhaps I should start phasing out NumberArray
instead.
NumberArray2-16
Vector2Like
-Matrix4Like