Skip to content

add Array.prototype.filter signature with type guard#10916

Closed
maiermic wants to merge 5 commits intomicrosoft:masterfrom
maiermic:type-guards-in-Array.prototype.filter-2
Closed

add Array.prototype.filter signature with type guard#10916
maiermic wants to merge 5 commits intomicrosoft:masterfrom
maiermic:type-guards-in-Array.prototype.filter-2

Conversation

@maiermic
Copy link

@maiermic maiermic commented Sep 14, 2016

10027 also didn't add the overload to Array#filter, only to ReadOnlyArray#filter. @mhegazy could you reopen this issue?

-- Arnavion's comment

This only adds it to ReadOnlyArray though. Also need it for regular Array.
-- Arnavion's comment

See #7657 and #10027

@msftclas
Copy link

msftclas commented Sep 14, 2016

Hi @maiermic, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@mhegazy
Copy link
Contributor

mhegazy commented Sep 14, 2016

thanks @maiermic, some baslines changed, you will need to do a gulp baseline-accept

mhegazy
mhegazy suggested changes Sep 14, 2016
View reviewed changes
Copy link
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

We will need to update the baselines as well. gulp baseline-accept will do the trick.

@maiermic
Copy link
Author

maiermic commented Sep 14, 2016

@mhegazy Thanks, I appreciate your help.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 14, 2016

waiting on the CI.

@maiermic
Copy link
Author

maiermic commented Sep 14, 2016 •
edited
Loading

@mhegazy Two tests failed on my machine. I ran gulp baseline-accept a second time and it modified two more files. I guess all tests will pass now, but this time I wait until my local test runner is done.

Update: Tests passed on my machine and I pushed my changes. Waiting on the CI again 😅

@Arnavion Arnavion mentioned this pull request Nov 13, 2016
Closed
@bcherny
Copy link

bcherny commented Dec 7, 2016

@mhegazy Is it possible to merge this?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 23, 2016

Sorry this fell off my list for a while. one more comment, we need to update the filter method on ReadOnlyArray, and the TypedArray varients, e.g. Int32Array (all in the same file).

@maiermic
Copy link
Author

maiermic commented Jan 18, 2017

@mhegazy This pull request only updates filter on Array. I updated filter on ReadOnlyArray in #10027 (already merged). TypedArray varients cann't use a type guard. They are not generic and contain always the same type. No update possible. So it should be possible to merge this.

@maiermic maiermic mentioned this pull request Jan 24, 2017
Closed
@ikokostya
Copy link
Contributor

ikokostya commented Mar 14, 2017

@mhegazy Could you review again taking into account latest comment #10916 (comment)?

@teppeis
Copy link

teppeis commented May 2, 2017

@maiermic @mhegazy any updates? I could resolve the conflicts and create new pr if you need.

Michael Maier added 2 commits May 4, 2017 07:02
Merge branch 'master' into type-guards-in-Array.prototype.filter-2
ebc8c94 
# Conflicts:
#	tests/baselines/reference/arrayFilter.symbols
#	tests/baselines/reference/arrayFilter.types
#	tests/baselines/reference/declarationEmitPromise.symbols
#	tests/baselines/reference/declarationEmitPromise.types
#	tests/baselines/reference/genericMethodOverspecialization.symbols
#	tests/baselines/reference/genericMethodOverspecialization.types
update array filter signatures with type guard
a811169
@maiermic
Copy link
Author

maiermic commented May 4, 2017

@teppeis Conflicts are resolved by my latest update. I'm waiting for @mhegazy to merge.

@NaridaL
Copy link
Contributor

NaridaL commented Jun 9, 2017

What's the status on this?

@mysticatea
Copy link

mysticatea commented Jul 17, 2017

@mhegazy @maiermic What is happening with this?

@maiermic
Copy link
Author

maiermic commented Jul 17, 2017

This has been solved by #16223 in the meantime.

@maiermic maiermic closed this Jul 17, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mhegazy mhegazy mhegazy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@maiermic @msftclas @mhegazy @bcherny @ikokostya @teppeis @NaridaL @mysticatea