observable icon indicating copy to clipboard operation
observable copied to clipboard

Should `first()` and `last()` support a default value?

Open domfarolino opened this issue 10 months ago • 7 comments

Just as an initial note to this issue: resolution of this issue is not required for the shipping of either first() or last() APIs.

It came to my attention through looking at the original WPTs for these APIs that there was an expectation of a "default" value for these APIs, in case they complete without emitting any values. RxJS appears to support this too (see https://rxjs.dev/api/operators/first & https://rxjs.dev/api/operators/last).

Personally I don't see this as necessary, since you could easily catch the case where completion happens without any values being emitted, and handle it yourself. But maybe the ergonomics is useful enough to include this? Either way, support for a default value could come at any point in the future too I think.

domfarolino avatar Apr 02 '24 22:04 domfarolino

The analog in RxJS (because promises) is actually firstValueFrom and lastValueFrom, which do support default values. As if they're empty they should reject, and that's not always desirable.

IMO, I think it's a strong add, and anecdotally, I've seen it used frequently

benlesh avatar Apr 04 '24 20:04 benlesh

Back when I did .NET, their enumerables had LastOrDefault and FirstOrDefault, which I thought were clear names and I remember using them often.

I do feel wary about getting ahead of iterator helpers for things like this, though.

domenic avatar Apr 04 '24 22:04 domenic

Iterator helpers are unlikely to get first or last methods at all, IMO. They don't seem clearly motivated to me for iterators and I don't think anyone's expressed interest in pushing for them.

bakkot avatar Apr 04 '24 22:04 bakkot

I do feel wary about getting ahead of iterator helpers for things like this, though.

Do you mean "getting ahead" by including these methods at all, in the first place?


Separately, I imagine first() (maybe less-so last()) would be used quite a bit for one-off events and would rarely depend on a default value being supplied, so changing the name to firstValueFrom or firstOrDefault feels kinda clunky for most really simple, cute use cases. Just from reading that I feel like I would double take upon every usage, wondering if I need to supply some input value, and having to cognitively process the rest of the words besides "first" to know if I'm doing the right thing. Maybe I'm just lazy though.

domfarolino avatar Apr 04 '24 23:04 domfarolino

Do you mean "getting ahead" by including these methods at all, in the first place?

Yes. Although if @bakkot thinks we're just not going to have this family for iterators, then maybe it's fine. I think in some ecosystems we'd push for consistency by including even useless methods, as long as they make sense. But oh well.

so changing the name to firstValueFrom or firstOrDefault feels kinda clunky for most really simple, cute use cases

Sorry, my suggestion was four methods: first(), firstOrDefault(defaultValue), last(), lastOrDefault(defaultValue). In JS you actually get a nice bonus where firstOrDefault() with no argument is equivalent to firstOrDefault(undefined), which seems helpful.

domenic avatar Apr 05 '24 02:04 domenic

I've thought about this a slight bit more and wanted to capture my thoughts.

Potential functionality you might want:

  • first/last/find, or undefined if empty/not found
  • first/last/find, or user-supplied default if empty/not found
  • first/last/find, or throw if not found

What iterator helpers/Array.prototype provides: find, or undefined if not found. So, let's stay consistent with that for Observable's find().

This leads to two choices to get the full functionality:

  1. If you want first/last to be consistent with find:

    • first() etc. return undefined if empty/not found.
    • first({ defaultValue }) etc., or firstOrDefault(defaultValue) etc., return defaultValue if empty/not found.
    • firstOrThrow() etc. throw if empty/not found
  2. If you don't want first/last to be consistent with find:

    • first() and last() throw if empty.
    • firstOrDefault(defaultValue) and lastOrDefault(defaultValue) return defaultValue if empty. (And automatically, firstOrDefault() returns undefined if empty.)
    • find() returns undefined if not found.
    • find({ defaultValue }) or findOrDefault(defaultValue) return defaultValue if not found.
    • findOrThrow() throws if not found.

We don't have to add anything beyond the first bullet point to start. Realistically, unless you are commonly dealing with observables containing undefined as a value, it is quite easy to get the other functionality by using fallback code. (E.g. observable.first().map(v => v ?? defaultValue).) But we should consciously choose between (1) and (2) as potential future paths. And those paths might spill back into iterator helpers or even Array.prototype.

From this perspective, I like (1) more.

domenic avatar May 04 '24 15:05 domenic

If you want first/last to be consistent with find: [...] From this perspective, I like (1) more.

Oh yeah, I'm personally very much sold on the path of being consistent with find(). For one, having to think of which methods throw when "the thing" is not found vs. not (or less scary, which methods respect defaults or otherwise have "default-consuming" counterparts) seems tricky to me.

I also like (1), and starting with the first bullet point is my preference for now. That basically means simplifying #131 and #144 to not use RangeErrors, and then keeping this issue open for future consideration of adding either a default-consuming counterpart method, or an optional default value parameter for first() and last(). So I think the "possible future enhancement" label for this issue is sufficient while work is done in the other PRs.

Thanks for the feedback, it's much appreciated!

domfarolino avatar May 04 '24 19:05 domfarolino