fix(typings): Use overloading instead of union type for typings filter, find, first, last.

[tetsuharuohzeki]

Description

• This fixes Type lost when using interfaces with filter, find, first, last #2163 by reverts commit 922d04e.
• So this reverts feat(filter, find, first, last): support type guard predicates that don't requiring casting #2119 sadly.

Drawback

• We would need specify type parameters to their methods explicitly if we'd like to narrow the type via predicate as type guarde function in some cases.
  • However, I don't think this is a serious problem because we can avoid this drawback with specifying actual types only.
  • And this changes makes our typings more similar to TypeScript's Array methods.

Related issue (if exists)

• Type lost when using interfaces with filter, find, first, last #2163
• feat(filter, find, first, last): support type guard predicates that don't requiring casting #2119

[coveralls]

Coverage remained the same at 97.686% when pulling 739593f on saneyuki:revert-2119 into 128fb9c on ReactiveX:master.

[tetsuharuohzeki]

We would need specify type parameters to their methods explicitly if we'd like to narrow the type via predicate as type guarde function in some cases.

You can see the diff

[tetsuharuohzeki]

@kwonoj @david-driscoll

How do you think about this?

[david-driscoll]

That was my thought for the solution as well.

I have one idea... I need to validate first, I'll check that right now.

[david-driscoll]

Nope, that didn't work sadly.

We can solve the guard case, or we can solve the much more common case. I would argue that it makes more sense to fix the common case.

LGTM 👍

[benlesh]

remove the tslint:disable comments, and actually use the variables with something like expect(), even if it's obvious like const x = 1; expect(x).to.equal(1)

[benlesh]

It's possible that a later version of TypeScript compiler might completely remove code related to unused variables as an optimization. I'm completely speculating, but it might be worth using expect() to set some obvious assertions around the variables you're not using, and remove these tslint:disable comments.

[rob3c]

Oops - sorry, guys! I only found out about this today, but I put together what I hope is a fix for this that'll let us keep the new support without breaking the previous behavior.

Please take a peek at #2170 before fully reverting it with this PR. I seem to have covered the concerns in #2163, but maybe there's something else that I missed.

[coveralls]

Coverage remained the same at 97.686% when pulling 739593f on saneyuki:revert-2119 into 128fb9c on ReactiveX:master.

[tetsuharuohzeki]

@Blesh @jayphelps

• I fixed what you reviewed.
• BTW, which do you accept this or Fix type guard additions #2170?
  • This pull request was opened before Fix type guard additions #2170
  • Fix type guard additions #2170 is opened by the original developer who write the patch which causes Type lost when using interfaces with filter, find, first, last #2163

[kwonoj]

@saneyuki , appreciate for effort for creating PR. as original author provided fix in #2170 and quick checking seems it correctly addresses problems, those PR has been checked in instead of taking it out. Let me close this PR in favor of #2170 . Thanks again!

[tetsuharuohzeki]

@kwonoj

OK. I agree to close this ;-)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.