Rxjs: feat(breaking): update startWith so that it will call function to get first value

Created on 22 Nov 2019  路  6Comments  路  Source: ReactiveX/rxjs

Feature Request

Ability/requirement to provide a callable function to startWith operator which is called to retrieve the first value.

I.e. update startWith interface from this:

startWith<T, D>(...array: Array<D | SchedulerLike>): OperatorFunction<T, T | D>

to this:

startWith<T, D>(...array: Array<(() => D) | SchedulerLike>): OperatorFunction<T, T | D>

Is your feature request related to a problem? Please describe.

I often find myself using the startWith function to create an observable which tracks the value of a property. i.e.

class Person {
  firstName = '';
  firstNameChanges = new Subject();
  firstName$ = this.firstNameChanges.pipe(startWith(this.firstName));
}

Unfortunately, this code doesn't behave as desired/expected, because the firstName$ property will always return "" as the first value (rather than whatever the current firstName is). I find myself wishing I could provide a function which always returns the current firstName as the first emission:

class Person {
  firstName = '';
  firstNameChanges = new Subject();
  firstName$ = this.firstNameChanges.pipe(startWith(() => this.firstName));
}

In this case however, startWith will return the function () => this.firstName as the first value, rather than calling the function and returning the result of the call as the first value.

Describe the solution you'd like

Personally, because of the behavior of map(fn: () => any), tap(fn: () => any), etc, I originally assumed startWith(fn: () => any) would call the provided function and use the returned value. While it's certainly possible that users would want to return an actual function as the "starts with" value, this use case seems much less common.

I'd like to see a breaking change so that startWith must be provided a function (or functions), and the return of this function would be used as the "starts with" value (I would argue that this should have been the API of the startWith operator to begin with, so this breaking change would be "fixing" a mistake--obviously this is just my opinion though). This would mean that, in order to provide a function as the "starts with" value, you would need to provide a function which returned a function (i.e. startWith(() => () => { /* do stuff */ })).

For consistency as well as proper type checking, I think it's important that users be forced to provide a function which returns a value for the startsWith() operator. I.e. current code like this startWith(0) would need to be changed to startWith(() => 0). This would allow IDEs to better type the arguments of startWith, and would make migrating userland code to the new API easier (simply identify all uses of startWith() in a repo, and wrap the existing uses with a function).

As an aside, I'm writing this issue after having spent at least 30-60 minutes debugging code using something like startWith(this.firstName) which was always returning the initial this.firstName value, rather than the current firstName value. I'm embarrassed to say this is not the first time I've made this exact mistake. Obviously this is 100% my fault, but the repeated occurrence of this mistake may reveal a design flaw with the current startWith API (obviously I think it reveals a design flaw).

  • Ideally, as an additional change, if the "starts with" function was called and returned a promise, the resolution of the promise would be used as the "starts with" value. I.e. startsWith(async () => true) would "start with" the value true. Though I'm not familiar with rxjs schedulars so this might not be possible with a single startsWith operator.

Describe alternatives you've considered

1. Let individual users define the desired startWith operator locally

Building off our previous example, individual users can currently achieve the desired functionality like so:

class Person {
  firstName = '';
  firstNameChanges = new Subject();
  firstName$ = this.firstNameChanges.pipe(
    startWith(null).pipe(map(() => this.firstName))
  );
}

Alternatively, they can define a new operator locally like so:

export function startWithRevised<T, R>(fn: (v: T) => R) {
  return startWith(null).pipe(map(fn));
}

I think both of these options are fine if the use case for startWith(null).pipe(map(() => this.firstName)) is rare. However, if I'm correct, and the need for startWith(null).pipe(map(() => this.firstName)) is very common, then there's an argument for baking the functionality into the startWith operator's API. Personally, I think the need for the new functionality is more common than the need for the existing functionality.

2. Use a BehaviorSubject

class Person {
  firstName$ = new BehaviorSubject('');
  firstNameChanges = this.firstName$.pipe(skip(1));
  get firstName() {
    return this.firstName$.value;
  }
}

Unfortunately, using a BehaviorSubject in this way is not always possible or straightforward.

For example, the most common use I have for startWith() is when extending an existing class / library which does not use rxjs.

For example, when extending an existing class you might attempt to do so using BehaviorSubject like so:

class PersonTwo extends Person {
  firstName$ = new BehaviorSubject('');
  firstNameChanges = this.firstName$.pipe(skip(1));
  get firstName() {
    return this.firstName$.value;
  }
  set firstName(val: string) {
    this.firstName.next(val);
  }
}

However, this workaround might be problematic if the underlying implementation is defined with firstName as a getter. For example, if the underlying implementation looks like so

class Person {
  private _firstName = '';
  get firstName() {
    return this._firstName;
  }
}

Then the PersonTwo code will not work as expected because the firstName setter will not be called.

(If this is new operator request) describe reason it should be core operator

If most people are like me and expect/desire the startWith operator to behave as described here, I think there is reason for the breaking change. If my experience is an outlier, then the preferred solution would be for individuals to simply use startWith(null).pipe(map(fn)) defer(() => this.aControl.valueChanges.pipe(startWith(this.aControl.value))) (from comment below).

Additional context

As a third party example of someone else potentially misusing the startWith operator (i.e., thinking the operator will perform as described here rather than how it actually works), you can see this issue in the Angular repo: https://github.com/angular/angular/issues/15282 (relevant text copied below).

Both FormGroup and FormControl provide .valueChanges, an observable stream of changes to the control or group of controls' value. This is very helpful, but it seems to omit the first value (before the first "change") for no particularly good reason other than consistency with the name.

This is easily worked around by with something like this:

const x$ = this.aControl.valueChanges.startWith(this.aControl.value);

... which is effective, but verbose and repetitive.

In fact, this "workaround" will always "start with" whatever the control's value was at the time x$ is defined, rather than whatever the control's value is when x$ is subscribed to. It's possible the author was aware of this distinction, but the description of the issue implies what the author wants was actually:

this.aControl.valueChanges.pipe(startWith(null).pipe(map(() => this.aControl.value)))

Or, with this proposal this.aControl.valueChanges.pipe(startWith(() => this.aControl.value))

Related

Issue #4113 is highly related to this request, in that it asked for the creation of a new operator to achieve the goals described here. Again, personally, the functionality described here is so common that I view the current startWith functionality as simply a bad API design which should be fixed with a breaking change to the current operator, rather than create a new operator.

4113 was closed with the comment that startWith(null).pipe(map(() => this.aControl.value)) could be done in user-land, however I argue that there is grounds to include any sufficiently common functionality in core, and startWith(null).pipe(map(() => this.aControl.value)) is sufficiently common.

Indeed, observable.pipe(startWith(0)) is already a "convenience function" for

concat(
  of(0),
  observable,
)

If this issue is closed without action, I'd like for the reason to be that folks don't think startWith(null).pipe(map(() => aValue))is sufficiently common functionality for inclusion in core (which, I concede, is a valid counter to this proposal), rather than simply "you can achieve this already" (which is obviously a somewhat hypocritical statement, as many core operators appear to be, on some level, merely convenience functions for things you could already achieve through other means).

Alternatively, another good counter argument to this proposal is "this is too big of a breaking change". In response to that, I'd say "fixing" any code effected by this change would be straightforward. Simply use an IDE's search to find existing uses of startWith() and wrap them with a function (ok, there might be a few uses which require a bit more effort to migrate, but hopefully the point is clear).

Most helpful comment

I'd suggest the following, instead:

firstName$ = defer(() => this.aControl.valueChanges.pipe(startWith(this.aControl.value)))

IMO, whenever you need something to happen to happen at subscription time, defer should be the first solution to consider.

All 6 comments

I'd suggest the following, instead:

firstName$ = defer(() => this.aControl.valueChanges.pipe(startWith(this.aControl.value)))

IMO, whenever you need something to happen to happen at subscription time, defer should be the first solution to consider.

@cartant I wasn't familiar with the defer function. That does look like a better solution than startWith(null).pipe(map(() => this.aControl.value)). Thanks!

However, I still think that the current startWith interface should be updated to expect a function. I think this updated API would conform better to how the operator is used, and, in general, make the operator more flexible.

Again though, I realize that this proposal is, currently, just one person's opinion and I may be an outlier (I'm hoping other folks seeing this issue may weigh in one way or another).

I am aware there are few ppl want to similar behavior, so don't think you are outlier. But given we have defer operator allow achieve this casually, I don't think we'll introduce this huge breaking changes. Even though change itself is straightforward, cost to whole application ecosystem is something carefully need to be considered.

This is same reason we have lot of utility fn for some surfaces - some are worth to be userland, but deprecating / breaking changes requires lot of consideration. For same reason, we encourage new feature to go userland module first.

Given surroundings, I'd suggest to use defer / or either publish userland operator wraps this behavior for immediate solution. We may discuss future of core behavior, but regardless of conclusion if it's breaking it'll take huge time.

@kwonoj makes sense. I definitely never expected this issue to warrant a new release by itself. However, the next time a major release is already happening, I think this issue is worth considering for inclusion.

Even though change itself is straightforward, cost to whole application ecosystem is something carefully need to be considered.

馃憤

I'd suggest to use defer / or either publish userland operator wraps this behavior for immediate solution.

I've currently decided to simply define my own startWith operator which performs as desired.

Closing as the general intention has explained and we will not make changes into core at this moment.

If this is closed, how will anyone remember to revisit it at a later date (e.g. 7.0) when changes to core are a possibility?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

peterbakonyi05 picture peterbakonyi05  路  4Comments

cartant picture cartant  路  3Comments

matthewwithanm picture matthewwithanm  路  4Comments

chalin picture chalin  路  4Comments

haf picture haf  路  3Comments