fix(router): Ensure routes are processed in priority order and only i… #38780
+36
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…f needed There is a slight difference between `map`...`concatAll` and `concatMap` in that the latter (`concatMap`) will ensure that the computations are executed in-order and only if needed while the former may execute the `map` body of all items if they do not emit immediately. That is, if the stream is `from([a, b, c]).pipe(map(v => of(v).pipe(delay(1))), concatAll(), first())` the `map` body will execute for all of `a`, `b`, and `c`. However, the following will only execute the `concatMap` body for `a` `from([a, b, c]).pipe(concatMap(v => of(v).pipe(delay(1))), first())` See https://stackblitz.com/edit/rxjs-cvwxyx fixes angular#38691
atscott
added
action: review
atscott
added a commit
to atscott/angular
that referenced
this pull request
Sep 9, 2020
This change contains the test from angular#38780. The fix is already present in master
atscott
added a commit
to atscott/angular
that referenced
this pull request
Sep 9, 2020
This change contains the test from angular#38780 and also removes `defer` from the `apply_redirects` logic because the change that introduced `concatMap` instead of `map`...`concatAll` makes `defer` unnecessary.
alfaproject
reviewed
Sep 10, 2020
atscott
added
action: merge
AndrewKushnir
pushed a commit
that referenced
this pull request
Sep 10, 2020
…f needed (#38780) There is a slight difference between `map`...`concatAll` and `concatMap` in that the latter (`concatMap`) will ensure that the computations are executed in-order and only if needed while the former may execute the `map` body of all items if they do not emit immediately. That is, if the stream is `from([a, b, c]).pipe(map(v => of(v).pipe(delay(1))), concatAll(), first())` the `map` body will execute for all of `a`, `b`, and `c`. However, the following will only execute the `concatMap` body for `a` `from([a, b, c]).pipe(concatMap(v => of(v).pipe(delay(1))), first())` See https://stackblitz.com/edit/rxjs-cvwxyx fixes #38691 PR Close #38780
|
This PR is now merged. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…f needed
There is a slight difference between
map...concatAllandconcatMapin that the latter (
concatMap) will ensure that the computations areexecuted in-order and only if needed while the former may execute the
mapbody of all items if they do not emit immediately. That is, if the streamis
from([a, b, c]).pipe(map(v => of(v).pipe(delay(1))), concatAll(), first())the
mapbody will execute for all ofa,b, andc.However, the following will only execute the
concatMapbody forafrom([a, b, c]).pipe(concatMap(v => of(v).pipe(delay(1))), first())See https://stackblitz.com/edit/rxjs-cvwxyx
fixes #38691
Note: rather than using
deferin thecanLoadcomputation, I went withconcatMapto avoid issues with missing thedeferin other places down the line. Note that this is the second time a bug has occurred because the computations of the route may happen immediately (the place I removeddeferfrom in this PR).Also note that
concatMapis already being used in themasterbranch but I will still need to create another PR to add the test from this one tomaster.