Skip to content

Correct and simplify core typed array typings #47503

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

Closed

Conversation

dead-claudia
Copy link

@dead-claudia dead-claudia commented Jan 19, 2022

Fixes #45198 #15402 #45199 #38665 and makes fixes for a number of other typed array typings issues a lot easier. May also reduce memory usage somewhat for apps not using the DOM typings.

This also makes a few other assorted fixes, like with the final parameter to methods like .find and .map, to align better with the spec.

This only touches typings. I ran into errors with paths being too long (it's not even all that heavily nested), so I wasn't able to run tests. (I created this using a sparse checkout to work around that, and so I'm also relying on CI here to check my work.)

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jan 19, 2022
@ghost
Copy link

ghost commented Jan 19, 2022

CLA assistant check
All CLA requirements met.

@sandersn sandersn self-requested a review February 16, 2022 01:25
@sandersn sandersn self-assigned this Feb 16, 2022
@gabritto
Copy link
Member

gabritto commented Mar 8, 2022

For reference: some of the changes in this PR seem to overlap with #47278

@dead-claudia
Copy link
Author

Note for reviewers: I did find this causes Node's Buffer#slice method to fail to type-check, but only the type definitions. If you disable lib checking, it'll still work as desired, but I felt this was worth calling out. (I tried doing this by adding an override for the slice as per this PR, so it might not reproduce from outright replacing it.)

@sandersn sandersn requested a review from gabritto March 10, 2022 18:13
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

If you want to keep working on this change, I'll start running Definitely Typed tests after all the baseline tests pass.

}

interface Int8ArrayConstructor {
new (elements: Iterable<number>): Int8Array;
Copy link
Member

Choose a reason for hiding this comment

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

When I run tests, I see a lot of failures because these constructors are no longer available. How are they supposed to be made available after this change?

Copy link
Author

@dead-claudia dead-claudia Jun 7, 2022

Choose a reason for hiding this comment

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

Whoops. I'll re-add them.

I've since gotten WSL hooked up, so I should be able to run the tests before pushing my update.

Edit: I also almost forgot about this PR. 😅

@sandersn
Copy link
Member

sandersn commented Aug 1, 2022

To help with PR housekeeping, I'm going to close this PR since it's pretty old now.

@sandersn sandersn closed this Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Big(Int|Uint)64ArrayConstructor: missing from Iterable
5 participants