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.

Sign up for GitHub

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

Jump to bottom

TypedArrays: fixed find and findIndex callback param obj type. #18493

Merged
merged 1 commit into from
Sep 18, 2017
Merged

TypedArrays: fixed find and findIndex callback param obj type. #18493

merged 1 commit into from
Sep 18, 2017

Conversation

NaridaL
Copy link
Contributor

@NaridaL NaridaL commented Sep 14, 2017 •
edited
Loading

Fixes #18425.

@msftclas
Copy link

@NaridaL,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

mhegazy
mhegazy reviewed Sep 14, 2017
View reviewed changes
src/lib/es5.d.ts
@@ -2644,7 +2644,7 @@ interface Uint16Array {
* @param thisArg If provided, it will be used as the this value for each invocation of
* predicate. If it is not provided, undefined is used instead.
*/
find(predicate: (value: number, index: number, obj: Array<number>) => boolean, thisArg?: any): number | undefined;
find(predicate: (value: number, index: number, obj: Int16Array) => boolean, thisArg?: any): number | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uint16Array ?

mhegazy
mhegazy reviewed Sep 14, 2017
View reviewed changes
src/lib/es5.d.ts
@@ -2655,7 +2655,7 @@ interface Uint16Array {
* @param thisArg If provided, it will be used as the this value for each invocation of
* predicate. If it is not provided, undefined is used instead.
*/
findIndex(predicate: (value: number, index: number, obj: Array<number>) => boolean, thisArg?: any): number;
findIndex(predicate: (value: number, index: number, obj: Int16Array) => boolean, thisArg?: any): number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uint16Array

mhegazy
mhegazy reviewed Sep 14, 2017
View reviewed changes
src/lib/es5.d.ts
@@ -3178,7 +3178,7 @@ interface Uint32Array {
* @param thisArg If provided, it will be used as the this value for each invocation of
* predicate. If it is not provided, undefined is used instead.
*/
find(predicate: (value: number, index: number, obj: Array<number>) => boolean, thisArg?: any): number | undefined;
find(predicate: (value: number, index: number, obj: Int32Array) => boolean, thisArg?: any): number | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uint32Array

mhegazy
mhegazy reviewed Sep 14, 2017
View reviewed changes
src/lib/es5.d.ts
@@ -3189,7 +3189,7 @@ interface Uint32Array {
* @param thisArg If provided, it will be used as the this value for each invocation of
* predicate. If it is not provided, undefined is used instead.
*/
findIndex(predicate: (value: number, index: number, obj: Array<number>) => boolean, thisArg?: any): number;
findIndex(predicate: (value: number, index: number, obj: Int32Array) => boolean, thisArg?: any): number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uint32Array

@NaridaL
Copy link
Contributor Author

NaridaL commented Sep 15, 2017

Yeah... sorry about that.

BTW, I tried it with the separate TypedArray interface, and it seems to be an improvement...

previous:

PS C:\Users\aval\tsdev\TypeScript> node built\local\tsc.js --diagnostics --p src\compiler\tsconfig.json
Files:            36
Lines:         93186
Nodes:        445312
Identifiers:  168285
Symbols:       71668
Types:         32088
Memory used: 371617K
I/O read:      0.02s
I/O write:     0.17s
Parse time:    0.71s
Bind time:     0.62s
Check time:    2.90s
Emit time:     2.06s
Total time:    6.29s

now:

PS C:\Users\aval\tsdev\TypeScript> node built\local\tsc.js --diagnostics --p src\compiler\tsconfig.json
Files:            36
Lines:         91325
Nodes:        440475
Identifiers:  166994
Symbols:       70992
Types:         31941
Memory used: 350637K
I/O read:      0.03s
I/O write:     0.03s
Parse time:    0.69s
Bind time:     0.58s
Check time:    2.84s
Emit time:     1.88s
Total time:    5.98s

... not sure if those statistics are the only relevant thing though.

@NaridaL
Copy link
Contributor Author

NaridaL commented Sep 15, 2017

Oh yeah, the issues with U/Intarray should be fixed now, because it's not immediately obvious from the code reviews^^.

@mhegazy mhegazy merged commit 8c2d79c into microsoft:master Sep 18, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Sep 18, 2017

We should be moving them out of es5.d.ts anyways. #15780 should be in first though.

@NaridaL NaridaL deleted the fix18425 branch September 25, 2017 07:47
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mhegazy mhegazy mhegazy left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NaridaL @msftclas @mhegazy