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

feat: Add sized array types #11

Merged
merged 5 commits into from
Jul 26, 2024
Merged

feat: Add sized array types #11

merged 5 commits into from
Jul 26, 2024

Conversation

ibgreen
Copy link
Contributor

@ibgreen ibgreen commented Jul 25, 2024

  • Add NumberArray2-16
  • Add Vector2Like - Matrix4Like

@coveralls
Copy link

coveralls commented Jul 25, 2024

Pull Request Test Coverage Report for Build 10115381086

Details

  • 56 of 114 (49.12%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 84.981%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/types/src/array-types.ts 0 58 0.0%
Totals Coverage Status
Change from base Build 10115283130: -0.2%
Covered Lines: 14612
Relevant Lines: 17343

💛 - Coveralls

number
];

/**
Copy link
Contributor

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?

Copy link
Contributor Author

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,
Copy link
Contributor

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

Copy link
Contributor Author

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.

@ibgreen ibgreen merged commit b09d004 into master Jul 26, 2024
1 check passed
@ibgreen ibgreen deleted the ib/sized-array-types branch July 26, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants