fix(delayWhen): no longer emits if duration selector is empty #5769
Conversation
This file contains 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
benlesh
force-pushed
the
fix/delayWhen-no-emit-on-empty-selector
branch
from
18f757e
to
85446f2
Compare
|
I am not sure if this is really a fix. I mean I get your point, but we had a test for the behavior when the delaySelector just completes. This could break people, right? |
|
This will resolve #3665. |
|
@niklas-wortmann Yes, this is a fix, which would be a breaking change, but it's still a bug fix and we want to get it into v7. |
- Resolves an issue where a duration selector that never emitted a value would cause the value to be emitted, even though there wasn't a "notification". - `delayWhen` is now based on `mergeMap`. I started off with trying to use `mergeInternals`, but quickly realized it was doing the same thing as `mergeMap`, and I could just use that directly. I'm not sure if this size optimization is "taking it too far", but I feel that `delayWhen` isn't exactly "hot path" code, so this optimization is fine, given the bundle savings it should yield. BREAKING CHANGE: `delayWhen` will no longer emit if the duration selector simply completes without a value. Notifiers must notify with a value, not a completion. fixes ReactiveX#3665
benlesh
force-pushed
the
fix/delayWhen-no-emit-on-empty-selector
branch
from
85446f2
to
611987d
Compare
|
Updated commit message. |
13 tasks
cartant
approved these changes
Oct 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
delayWhenis now based onmergeMap. I started off with trying to usemergeInternals, but quickly realized it was doing the same thing asmergeMap, and I could just use that directly.I'm not sure if this size optimization is "taking it too far", but I feel that
delayWhenisn't exactly "hot path" code, so this optimization is fine, IMO, given the bundle savings it should yield.