Platform: Strange withLatestFrom behavior
Minimal reproduction of the bug/regression with instructions:
Lets have very common Effect + Selector code:
// effect.ts
@Effect()
effect1$: Observable<Action> = this.actions$.pipe(
ofType<XYZAction>(Actiontypes.XYZAction),
withLatestFrom(this.store.pipe(select(anySelector))),
tap(..),
map(...)
);
// selector
export const getFeatureState= createFeatureSelector('feature1'); // 'router' actually
// map callback is called before any action... << correct behavior?
export const anySelector= createSelector(getFeatureState, (x: any) => x.a.b.c.d); // x.state.root actually
Expected behavior:
withLatestFrom should take last value from this.store when and ONLY WHEN the first observable emits value (XYZAction dispatched). However... selector is called before any action..
I think this should work, because i am subscribing to store/select/selector only when action is dispatched (and taking only 1 - take(1))
From my point of view it behaves more like combineLatest than withLatestFrom...
Is this the way it should behave? I got NGRX/Effects are subscribed to effect -> selector, but i wonder if it is documented well... Thank you.
(btw. state from store is routerstate from ngrx/router)
Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):
Angular CLI: 7.3.1
Node: 10.9.0
OS: win32 x64
Angular: 7.2.12
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router
Package Version
@angular-devkit/architect 0.12.4
@angular-devkit/build-angular 0.12.4
@angular-devkit/build-ng-packagr 0.12.4
@angular-devkit/build-optimizer 0.12.4
@angular-devkit/build-webpack 0.13.8
@angular-devkit/core 7.2.4
@angular-devkit/schematics 7.3.1
@angular/cli 7.3.1
@angular/pwa 0.12.4
@ngtools/json-schema 1.1.0
@ngtools/webpack 7.2.4
@schematics/angular 7.2.4
@schematics/update 0.13.1
ng-packagr 4.7.1
rxjs 6.3.3
typescript 3.2.2
webpack 4.28.4
@ngrx/effects": "^7.3.0",
"@ngrx/router-store": "^7.3.0",
"@ngrx/store": "^7.3.0",
"@ngrx/store-devtools": "^7.3.0",
montella1507
All 12 comments
withLatestFrom should take last value from this.store when and ONLY WHEN the first observable emits value (XYZAction dispatched). However... selector is called before any action..
withLatestFrom would combine the values only when action is dispatched, however it starts listening for the this.store.pipe(select(anySelector)) right away, so this is the expected behavior unrelated to NgRx.
alex-okrushko
on 8 Apr 2019
(btw. state from store is routerstate from ngrx/router)
@montella1507, are you getting the error somewhere, because it's undefined? There are solutions for that, if that's the case.
alex-okrushko
on 8 Apr 2019
@alex-okrushko well.. so it emits final value ONLY WHEN FIRST observable emits value ($actions.oftype()) but it subscribe to second one (store) right away... right?
So:
export const anySelector= createSelector(getFeatureState, (x: any) => x?.a?.b?.c?.d); // of course with if (null checkx)
is the only solution.. -> " Do not rely that subscriber is subscribed only after action is dispatched".
So.. if there will be no this.store in withLatestFrom, but HTTP observable.. it will call server before action.?
montella1507
on 8 Apr 2019
A few solutions (for ngrx/router case):
- Provide the initial state for the
ngrx/router- this is my recommended solution - Add checks in the the selector
- instead of
withLatestFrom(store.select(...)),do something likeconcatMap(action => of(action).pipe(withLatestFrom(store.select(...)))),
So.. if there will be no this.store in withLatestFrom, but HTTP observable.. it will call server before action.?
Yes, it will, that's one of the reasons why we concatMap into the service call.
alex-okrushko
on 8 Apr 2019
I filed a new issue in #1714 to cover this in the docs
brandonroberts
on 8 Apr 2019
@brandonroberts thank you very much! withLatestFrom is directly used after ofType almost in every tutorial... unfortunately.
montella1507
on 8 Apr 2019
@alex-okrushko so this may be the solution as well:
function withLazyLatestFrom<TSecond>(second: Observable<TSecond>) {
return function latestFromIsNotSoObvious<TSource>(source: Observable<TSource>) {
return Observable.create(subscriber => {
const subscription = source.pipe(
concatMap(action =>
of(action).pipe(
withLatestFrom(second)
)
)).subscribe(
value => {
try {
subscriber.next(value);
} catch (err) {
subscriber.error(err);
}
},
err => subscriber.error(err),
() => subscriber.complete());
return subscription;
}) as Observable<[TSource, TSecond]>;
}
}
// effect
@Effect()
effect1$: Observable<Action> = this.actions$.pipe(
ofType<XYZAction>(Actiontypes.XYZAction),
withLazyLatestFrom(this.store.pipe(select(anySelector))),
+ i dont need to "cast" tuple anymore.
@montella1507 Interesting that you mentioned that, because I have the operator that buffers incoming action until the store selector returns non-null value - meaning something was set and it's not the initial one anymore:
export function withBufferedLatestNonNull<T>(
...observables: Array<Observable<any>>): <T>(source: Observable<T>) =>
Observable<T[]> {
return <T>(source: Observable<T>) => {
// Create a new Observable for referential transparency.
return new Observable<T[]>(observer => {
const subscription = new Subscription();
// Create Replayable subjects and connect right away, so that
// emission from hot Observables are collected
const connectables$ = observables.map(o$ => {
const connectable = publishReplay(1)(o$) as ConnectableObservable<any>;
subscription.add(connectable.connect());
return connectable;
});
subscription.add(
source
.pipe(
// Used mergeMap instead of combineLatest directly because we
// want to chain all the source incoming values instead of
// dropping them.
mergeMap(
sourceValue =>
combineLatest(of(sourceValue), ...connectables$)
.pipe(
// Take the only when ALL values are non null.
filter(
result => result.every(
(v: any): v is NonNullable<any> =>
v != null)),
// Ensures that new values are not picked up,
// once we have the combination.
first(),
),
),
)
.subscribe(observer));
return subscription;
});
};
}
I would still recommend setting the initial value for router state
alex-okrushko
on 9 Apr 2019
Crazy that this has caused problems for people :o I've yet to have any issues with what was described here, maybe because that's how I architect my stores/actions/effects?
I had found the note in the new v8 docs about this and wanted to track down why this was being suggested, good job getting it added! 馃憤
Also if commenting on old closed issues is a no-no just lemme know and I'LL SHOW MYSELF THE DOOR 馃毆 馃憟 kthxbaaiiii
jsonberry
on 8 Jun 2019
@jsonberry it shows up in my notifications, so it's completely fine to comment on closed issues 馃槀
This case is very frequent in Firebase Console, mostly because we have router data as part of the store. For example, I'm navigating to one of the pages and the project details are used to query the info needed for that page. Project details are not available yet - I have only the project id as part of the url, so I need to defer the effect until store has project details data in it (until project details effect pushes the success data into the store).
alex-okrushko
on 8 Jun 2019
@alex-okrushko ah okay, absolutely, I read over the issue conversation more and it all makes sense. I'm a little puzzled by your example so I want to just clarify some things.
_I'm navigating to one of the pages and the project details are used to query the info needed for that page_
鈽濓笍 When you say query, are you referring to a selection from the store, or something like a network call for data? Are you using details of an entity in order to make subsequent network requests (more than the entity ID)?
Does this imaginary setup somewhat match your example?
@Effect()
entityDetailRoutedEffect$ = this.actions$.pipe(
ofType(ROUTER_NAVIGATED),
withLatestFrom(this.store.pipe(select(routerQuery.getUrl))),
filter(([, url]) => url.startsWith('/entity/')),
mapTo(fetchEntityDetails)
)
@Effect()
fetchEntityDetailsEffect$ = this.actions$.pipe(
ofType(fetchEntityDetails),
withLatestFrom(this.store.pipe(select(routerQuery.getParamId))),
switchMap(([, id]) => this.s.fetch(...))
)
鈽濓笍 If those selectors aren't setup with null checks or the router store isn't set up with an initialState, the effects would blow up immediately on boot. The proposed fix in docs is to wrap them:
ofType(ROUTER_NAVIGATED),
concatMapTo(withLatestFrom(this.store.pipe(select(routerQuery.getUrl)))),
ofType(fetchEntityDetails),
concatMapTo(withLatestFrom(this.store.pipe(select(routerQuery.getParamId)))),
I can see how the same issue would arise with similar selectors for store data unrelated to the router-store, but the wording in the docs and what's proposed don't seem to match with what I would expect. I would have expected the wording to be a bit more about null checks and less about performance, and the proposed solution perhaps could have been to use a defer.
Current suggestion in v8.0.0 docs:
concatMap(action => of(action).pipe(
withLatestFrom(this.store.pipe(select(fromBooks.getCollectionBookIds)))
)),
I'd imagine something like this to be more accurate for the problem:
withLatestFrom(defer(() => this.store.pipe(select(fromBooks.getCollectionBookIds))))
鈽濓笍 Is my understanding of the problem and the proposed solution accurate? If so I can open a separate issue/PR for the docs to move the conversation to an appropriate place.
jsonberry
on 8 Jun 2019
I use this:
import { Observable, of } from 'rxjs';
import { concatMap, withLatestFrom } from 'rxjs/operators';
export function withLatestCached<TSource, TSecond>(second: (input: TSource) => Observable<TSecond>) {
return function latestFromisBad(source: Observable<TSource>) {
return Observable.create(subscriber => {
const subscription = source
.pipe(
concatMap(action =>
of(action).pipe(
withLatestFrom(second(action))
)))
.subscribe(
value => {
try {
subscriber.next(value);
} catch (err) {
subscriber.error(err);
}
},
err => subscriber.error(err),
() => subscriber.complete()
);
return subscription;
}) as Observable<[TSource, TSecond]>;
};
}
with usage (all is stongly typed + Input observable is mapped to "second observable" - you cannot use "action" easily as payload to withaLatestFrom / or flattened withlatestfrom) :
montella1507
on 8 Jun 2019
Related issues
RichardMisiak
路
3Comments
brandonroberts
路
3Comments
sandangel
路
3Comments
alvipeo
路
3Comments
itprodavets
路
3Comments
Most helpful comment
@montella1507 Interesting that you mentioned that, because I have the operator that buffers incoming action until the store selector returns non-null value - meaning something was set and it's not the initial one anymore:
I would still recommend setting the initial value for router state