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

Simplify types of array/slice and array/base/slice via use of generic type parameter #1318

Merged
merged 5 commits into from
Feb 21, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Apply suggestions from code review
Signed-off-by: Athan <kgryte@gmail.com>
  • Loading branch information
kgryte authored Feb 21, 2024
commit e1d0be834a14ed9d7f8ce4d70901de29e7dfc8de
Original file line number Diff line number Diff line change
@@ -65,8 +65,8 @@ declare function slice<T extends TypedArray | ComplexTypedArray>( x: T, start: n
* @example
* var x = [ 1, 2, 3, 4, 5, 6 ];
*
* var out = slice( x, 0, 2 );
* // returns [ 1, 2 ]
* var out = slice( x, 0, 3 );
* // returns [ 1, 2, 3 ]
*/
declare function slice<T = unknown>( x: Collection<T>, start: number, end: number ): Array<T>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to use the interface approach I mentioned previously, or should we be content with typing like this? Atm, this will return incorrect results for specialized constructors having a slice method which are not captured by TypedArray | ComplexTypedArray. Granted, the previous declarations also did not capture this case, but we could go ahead and correct here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kgryte We should merge this PR without the proposed change as I am not getting it to work properly with tuple types. Have some work-in-progress stuff we can discuss but this is quite complicated and would also require some opaque typing, I believe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. We should create a TODO to follow-up.


Loading