is there a TECHNICAL reason this approach is flawed
Yes. As @frosty points out, you could run into a race condition when getItems() is executed multiple times, since you are storing data "outside of the stream", the state of this.myItem is dependent on the order in which your http requests return.
While this may work most of the time, it is not completely deterministic.
I am getting lost converting the trivial example into something that applies to me
I get it. RxJS is hard... at first :-)
One thing that helped me a lot in becoming proficient is to realize that:
- Observables on their own, are pretty boring
- RxJS makes working with them worthwhile
- this is because there are so many operators and static functions that allow you easily create observable sources with well defined behavior.
- There are essentially 2 features of an observable: what and when
- what is the shape of data it emits ?
- when will it emit this data ?
- You can break observables down into smaller parts, which makes understanding and debugging much easier!
Let's take your initial example code (note: for sake of clarity for future readers I've renamed projectRev to item):
export class SomeComponent {
public myItem : Item;
public myProject : Project;
ngOnInit() {
this.getItem();
}
getItem() {
let itemId = this.activatedRoute.snapshot.params.id;
this.itemSvc.getItemById(itemId).subscribe(
data => {
this.myItem = data;
this.getProject();
}
);
}
getProject() {
this.projectSvc.getProjectById(this.myItem.ProjectId).subscribe(
data => this.myProject = data
);
}
}
Let's design a single observable that emits exactly the data you want, exactly when you want it!
Thinking of this ahead of time makes life much easier.
For the sake of example, let's say you want to emit an Item with its parent Project attached. So,
- what :
Item object with parent Project appended
- when : should emit whenever the source
item is changed (id is different)
To accomplish this, we can define all the individual parts as separate observables. Angular provides the route params as an observable, so rather than using .snapshot which represents the state at one moment in time, let's define an itemId$ observable that will emit when the param changes:
this.itemId$ = this.activatedRoute.params.pipe(pluck('id'));
Let's also define myItem as observable. We would like myItem$ to emit the current Item (what), whenever the id route parm changes (when):
this.myItem$ = this.itemId$.pipe(
switchMap(itemId => this.itemSvc.getItemById(itemId))
);
At first, switchMap may seem confusing (I know it was for me). Here's what it does:
- it internally subscribes to an observable source and emits its emissions
- each time it receives a new emission, it will stop listening to the previous source and subscribe to a new source
- in your case, we provided a function that takes the received emission from
itemId$ and returns an observable. This observable is the call to this.itemSvc.getItemsById()
So, hopefully you can see that whenever itemId$ emits an id, myItem$ will emit the result of itemSvc.getItemById(), which is the Item object.
Notice, there is no subscription (this is handled internally by switchMap). Notice there is no need to stash the result in a separate local variable this.myItem, which was the cause of your possible race condition.
Next, let's define an observable that emits our Item with an additional project property (what) whenever a new Item is emitted (when):
For the sake of verbosity:
this.myItemWithProject$ = this.myItem$.pipe(
switchMap(item => this.projectSvc.getProjectById(item.ProjectId).pipe(
map(project => ({ ...item, project }))
))
);
Here we defined myItemWithProject$ as an observable that begins whenever myItem$ emits, then used our new friend switchMap to make a call to get the parent Project. We then use map to simply return a copy of the Item object with an additional project property.
Here's a StackBlitz that shows this altogether.
Maybe you don't want a single combined object, you could obviously shape the data any way you want, maybe a single ViewModel object that has item and project properties. This is actually pretty common in Angular:
combineLatest is a great operator to handle this for you:
public vm$ : Observable<ViewModel> = combineLatest({
item : this.myItem$,
project : this.myProject$}
);
This approach allows you to use a single observable in your template and a single async pipe to unwrap it:
<div *ngIf ="vm$ | async as vm">
<h2> My Item </h2>
<p> {{ vm.item | json }} </p>
<h2> My Project </h2>
<p> {{ vm.project | json }} </p>
</div>
As your component becomes more complex, you can simply add more sources to the vm$:
public vm$ : Observable<ViewModel> = combineLatest({
item : this.myItem$,
project : this.myProject$,
source1 : this.source1$,
source2 : this.source2$
});
StackBlitz #2
Keeping it "observable all the way" can make things really concise and tidy. But, it requires that you understand what the operators are actually doing (the what and when).
It's usually not necessary to stash data outside of the observable stream. I find that when we reach for that as a solution, it's because we don't yet fully understand all of the operators provided to us by rxjs.
Am i right in thinking i should be doing concatMap instead of switchMap
The difference between the higher order mapping operators only comes into play when they receive more than one emission. This is because they all subscribe to inner sources and emit their emissions. The difference is the strategy they use when a new emission is received before the current source completes:
switchMap "switches" sources to only emit from the most recent source.
exhaustMap will ignore new emissions until the current source completes, so it only emits from the first source.
mergeMap will emit from all sources old and new.
concatMap is really just a special case of mergeMap where it will only allow one concurrent source at a time, but will eventually emit from all sources
So, in your case, I think switchMap is appropriate, because if the id changes, you no longer care about listening to emission about object with old ids.
