Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Complete notifications deemed to be signals #5838

Closed
8 tasks done
cartant opened this issue Oct 20, 2020 · 1 comment
Closed
8 tasks done

Complete notifications deemed to be signals #5838

cartant opened this issue Oct 20, 2020 · 1 comment
Assignees
@cartant

Comments

@cartant
Copy link
Collaborator

cartant commented Oct 20, 2020 •
edited
Loading

Bug Report

With numerous operators, the next and complete notifications received from notifier (or selector) observables are treated in the same way. As commented here, that is arguably incorrect behaviour (at the very least, IIRC, this behaviour is inconsistent throughout the API):

// TODO(benlesh): I'm inclined to say this is _incorrect_ behavior.
// A completion should not be a notification. Note the deprecation above

AFAICT, the following operators effect this incorrect behaviour:

For more context, this was checked off in the list of non-type-related things that need to be done before v7 can be released, but it is not that case that all of these behaviours have been corrected. See #5372
@cartant cartant mentioned this issue Oct 20, 2020
Closed
13 tasks
@cartant cartant added AGENDA ITEM Flagged for discussion at core team meetings and removed AGENDA ITEM Flagged for discussion at core team meetings labels Oct 20, 2020
@cartant
Copy link
Collaborator Author

cartant commented Oct 25, 2020

Removed the 'Agenda Item' label. In last week's meeting it was decided that these operators should be changed so that the behaviour is consistent and complete is not treated as a signal.

@cartant cartant self-assigned this Oct 25, 2020
@cartant cartant mentioned this issue Oct 25, 2020
Merged
cartant added a commit to cartant/rxjs that referenced this issue Oct 27, 2020
@cartant
fix(windowToggle): don't signal on complete
66a8b05 
Closes ReactiveX#5838
cartant added a commit to cartant/rxjs that referenced this issue Oct 31, 2020
@cartant
fix(windowToggle): don't signal on complete
bd00e37 
BREAKING CHANGE: the observable returned by the windowToggle operator's
closing selector must emit a next notification to close the window.
Complete notifications no longer close the window.

Closes ReactiveX#5838
cartant added a commit to cartant/rxjs that referenced this issue Oct 31, 2020
@cartant
fix(windowToggle): don't signal on complete
a06f305 
BREAKING CHANGE: the observable returned by the windowToggle operator's
closing selector must emit a next notification to close the window.
Complete notifications no longer close the window.

Closes ReactiveX#5838
cartant added a commit to cartant/rxjs that referenced this issue Oct 31, 2020
@cartant
fix(windowToggle): don't signal on complete
b79b348 
BREAKING CHANGE: the observable returned by the windowToggle operator's
closing selector must emit a next notification to close the window.
Complete notifications no longer close the window.

Closes ReactiveX#5838
cartant added a commit to cartant/rxjs that referenced this issue Nov 2, 2020
@cartant
fix(windowToggle): don't signal on complete
8fcbc6c 
BREAKING CHANGE: the observable returned by the windowToggle operator's
closing selector must emit a next notification to close the window.
Complete notifications no longer close the window.

Closes ReactiveX#5838
@benlesh benlesh closed this as completed in 9cb56c4 Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cartant cartant

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@cartant