feat(some): Observable<T>.some()

[Pappa]

Description:

Implementation of "some" operator with tests.

Related issue (if exists):

[coveralls]

Coverage increased (+0.02%) to 97.056% when pulling 4592a91 on Pappa:some-operator into 8ebee66 on ReactiveX:master.

[kwonoj]

I sometimes use this operator and in v5 just using reduce instead, so bit +1 to add but need clarification if this is good to be added in api surface. /cc @Blesh @jayphelps

[kwonoj]

v5 drops support of thisArg except native interface has thisArg to match it. (#878)

[Pappa]

Hi @kwonoj,

some and every are both native operators. #878 doesn't mention them explicitly, but should they both support a thisArg parameter if implemented in RxJS?

[kwonoj]

Uh-oh.. yes, my bad.

[jayphelps]

@kwonoj isn't using reduce not the same, since it exhausts the source until natural complete whereas some completes early if it finds a matching condition?

I may be missing a more obvious solution that is identical behavior to some, but this is one:

interval(1000)
  .filter(x => x % 2 === 0)
  .mapTo(true)
  .take(1)
  .defaultIfEmpty(false)
  .subscribe(value => console.log(value));

Which is extremely verbose and confusing..I agree it comes down to whether or not we want to increase the API surface of core for an operator I feel a majority of people won't use--though when they need it, it may not be super obvious how to simulate it using existing operators or creating a custom one-off observable like:

const item$ = interval(1000);

new Observable(observer => {
  return item$.subscribe({
    next: x => {
      if (x % 2 === 0) {
        observer.next(true);
        observer.complete();
      }
    },
    complete: () => {
      observer.next(false);
      observer.complete();
    }
  })
});

All things considered, right now I'm personally 👎 to adding some to core, until we get v5 out and more concretely figure out where these less common operators belong, like some kitchen sink addon or namespace. We can discuss this on Monday at our meeting though, I could be convinced 😄

[kwonoj]

Yes, using reduce is not same. I only use it cause I know how many source element stream will it be and certain it won't be many, so there is no strong difference between more-legit to just reduce.

That's reason I did +1 but 'bit', it's generally doable with combining exist operator but not straightforward manner, but also I question usefulness of operator itself as core (how much this operator being used? Is it significantly important?) As well - reason not strongly suggest to expand api surface to include this.

[benlesh]

This is just source.first(predicate).mapTo(true).defaultIfEmpty(false) is it not?

[benlesh]

There's a typo on that last one because I'm one my phone. LOL.

[jayphelps]

@Blesh yes, though how to do it that way is probably not obvious for newcomers (heck, I forgot first() accepts a predicate and used filter + take(1) ) and it's obviously not as clear as just some is, which as you know people may be familiar with because of Array#some. I'm still 👎 though, just clarifying what I believe is the argument for it.

[mattpodwysocki]

@jayphelps @Blesh seems like a hack to not have the basic Array extras even if Array.prototype.some and Array.prototype.every both need completion semantics in order to work, but then again, if we're going to have reduce why not have some and every and includes?

[Pappa]

In MIGRATION.md some is labelled as "Not yet implemented". I assumed that means there is an intention to implement it. If it's not the case that all operators marked "Not yet implemented" are desired/needed, perhaps that document should be changed.

I agree with @mattpodwysocki above, it seems odd to include every but not some.

[jayphelps]

@Pappa We met today and discussed this PR. (notes here).

We want to continue reduce the size of the core set of operators, which might even mean removing some that already exist. We do agree there is a place for this, and will continue to discuss the prospect of optional addons. There are many possibilities of what they would look like, but for now we want to keep our focus on shipping v5.0.0-final as soon as possible and will come back to this shortly after.

At that time, we'll undoubtedly need some changes to this PR to adapt. Do you want to keep this PR open until then or close for now? If you have extra cycles, we'd be open to someone spearheading the addon effort and would love your help if you're interested.

[Pappa]

@jayphelps It's fine to close the PR for now.

I'll play around with v5 for a while and help fix some bugs, to get a feel for the library internally, and if I have time, try to come up with some ideas around addons and follow any discussion you have on this.

[paulpdaniels]

@jayphelps now that 5.0.0 is out and about, is it time to revisit this discussion (in a general sense) or does that already exist somewhere I'm not seeing.

[jayphelps]

@paulpdaniels @mattpodwysocki is taking a crack at formalizing our RFC process document in #2299. There's still a general reluctance to adding more operators that aren't used a lot and are not difficult to do with composition of existing ones.

At Rx Contributor Days yesterday we heard again that one of the biggest complaints from community members who are learning is that there are too many operators and it's confusing. We recorded the discussion and will likely follow up in our core team notes, but I imagine we'll prolly do a combination of improving documentation by grouping operator docs in some fashion, removing operators that are not frequently used and easy to implement, and considering maintaining addon packages outside of the core rxjs package. Nothing is set in stone, and it's hard to say when/if we'll be able to..we really hope to have some community help. e.g. if we could figure out an addon package system that was maintainable we could then not fear adding a large number of "nice to have" operators in those addons. We originally thought using operator patching would be the solution, but rxjs is very popular in the Electron world where they do not bundle and require() file IO becomes a problem.

We'd love to hear your thoughts on the matter of course!

[kwonoj]

As same as #1950, I'm going to close this for inactive discussion along with our recommendation to create operator as a separate module. Thanks @Pappa for contribution, and apologies for makes PR hanging without getting decision quickly.

[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.