refactor: smallify all the things#5729
Merged
benlesh merged 140 commits intoReactiveX:masterfrom Sep 23, 2020
Merged
Conversation
- Smaller implementation - Improved docs - Adds comments - Adds a test - Fixes weird adjustment in `reduce`. Closes ReactiveX#4348 Closes ReactiveX#3879
- Refactor to make it smaller - Replaced a faulty test with a test that tests the same thing. Verified the behavior has not changed since 6.x.
- Centralize all try/catching for next calls.
- map is smaller
- Uses OperatorSubscriber - Much smaller
- Smaller implementation - Uses OperatorSubscriber
- Removes runtime assertions
- Adds supporting features to OperatorSubscriber
…new window when old one closes This was a long-broken bit of functionality, but windowTime was so little used, I chose to ignore it. With this change, we could make `bufferTime` based off of `windowTime` and `toArray`.
- Does a little work to make sure that we don't push onto the buffer unless we have to.
Member
Author
|
@cartant and @jayphelps ... I think I've addressed all of your comments. I've unified on an approach for freeing up resources like buffered values, I think... basically, a returned function from the lift is a terse way to add a teardown to the destination subscriber. This ensures that the resource will be freed regardless of source (or inner) completion or error, or destination unsubscription. It could delay the release of those resources in some cases, so things will still have to be addressed on a case-by-case basis. But over all it looks like this: return (source: Observable<T>) => lift(source, function(this: Subscriber<R>, source: Observable<T>) {
const subscriber = this;
/* stuff here */
// Additional teardown
return () => {
buffer = null!
};
});This is equivalent to making the last line: |
cartant
reviewed
Sep 22, 2020
cartant
reviewed
Sep 22, 2020
cartant
reviewed
Sep 22, 2020
cartant
reviewed
Sep 23, 2020
jakovljevic-mladen
added a commit
to jakovljevic-mladen/rxjs
that referenced
this pull request
Sep 28, 2020
jakovljevic-mladen
added a commit
to jakovljevic-mladen/rxjs
that referenced
this pull request
Oct 3, 2020
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Going through, operator by operator, and making them smaller. Centralizing on a new type called
OperatorSubscriberthat enables most behaviors fairly cleanly and concisely.WIP
Will show stats when finished.
Progress Update 9/17
BEFORE: 93KB
AFTER: 59KB (63% of the original size)AFTER: 57KB (61% of the original size)
Review Checklist:
Reviewed files:
Check only files that have been reviewed and have no issues. If there are issues with a review, add a link to the review to the listed file.