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

Issue/5739 smaller operator creation #5742

Merged

benlesh merged 12 commits into ReactiveX:master from benlesh:issue/5739-smaller-operator-creation

Sep 23, 2020

Member

benlesh commented Sep 23, 2020 •

edited

Loading

Adds new operate function for creating operators
Replaces all lift and wrappedLift helpers.
Fixes an issue with publishReplay et al, where an error thrown in the selector function was being thrown externally instead of send to the subscriber via an error notification.

Further size reduction.

~~57K -> 55K~~
57K -> 54K approximately

benlesh commented

View reviewed changes

src/internal/util/lift.ts Outdated Show resolved Hide resolved

benlesh commented

View reviewed changes

src/internal/operators/map.ts Show resolved Hide resolved

benlesh commented

View reviewed changes

spec/operators/publishReplay-spec.ts Show resolved Hide resolved

benlesh commented

View reviewed changes

src/internal/operators/multicast.ts Show resolved Hide resolved

benlesh commented

View reviewed changes

src/internal/operators/withLatestFrom.ts Show resolved Hide resolved

benlesh requested a review from cartant

September 23, 2020 06:02

cartant requested changes

View reviewed changes

Collaborator

cartant left a comment

LGTM, but there are a bunch of return statements in the operators that I think aren't needed. And I have a few nitpicks.

src/internal/util/lift.ts Outdated Show resolved Hide resolved

src/internal/operators/bufferToggle.ts Outdated Show resolved Hide resolved

src/internal/operators/combineLatestWith.ts Show resolved Hide resolved

src/internal/operators/count.ts Outdated Show resolved Hide resolved

src/internal/operators/filter.ts Outdated Show resolved Hide resolved

src/internal/operators/takeWhile.ts Outdated Show resolved Hide resolved

src/internal/operators/tap.ts Outdated Show resolved Hide resolved

src/internal/operators/raceWith.ts Show resolved Hide resolved

src/internal/operators/zipWith.ts Outdated Show resolved Hide resolved

src/internal/operators/refCount.ts Outdated Show resolved Hide resolved

tonivj5 reviewed

View reviewed changes

src/internal/util/lift.ts Outdated Show resolved Hide resolved

benlesh requested a review from cartant

September 23, 2020 15:21

Member Author

benlesh commented Sep 23, 2020

HA! While fixing the stuff @cartant pointed out, I updated onErrorResumeNext, retry and repeat, and knocked us down another 1K almost.

cartant approved these changes

View reviewed changes

benlesh added 11 commits

September 23, 2020 14:54


          refactor: Create new operate internal operator creation function

3f87e7d

resolves ReactiveX#5739


          fix(multicast): and other publish variants will handle errors thrown …

a780e97

…in a selector appropriately

- Also refactors multicast to use the new `operate` mechanism.


          refactor: remove last uses of wrappedLift

258a3a2


          refactor: remove unused lift and wrappedLift functions.

34662e0


          refactor(onErrorResumeNext): use OperatorSubscriber, don't return sub…

7b6b239

…scription in operate.


          refactor(repeat): Use OperatorSubscriber, don't return subscription…

608a159

… in `operate`


          refactor(retry): Use OperatorSubscriber, don't return subscription …

ea0179a

…in `operate`.


          refactor: Don't return subscriber or subscription in operate unnece…

d85f67f

…ssarily.


          refactor: use optional chaining

c056a40


          refactor(operate): Tweak init return type so Subscribers and Subscrip…

ee7de5d

…tions can no longer be returned

This will help force us to make sure we are using the subscriber and subscription chaining in the most efficient way possible. Although it could result in anti-patterns where users return a function that calls unsubscribe on a subscription if we release this to the public.


          refactor(bufferToggle): rename liftedSource to source.

7c694b6

benlesh force-pushed the issue/5739-smaller-operator-creation branch from f73e056 to 7c694b6 Compare

September 23, 2020 19:54


          chore: fix lint

adac67b

benlesh merged commit 31fd513 into ReactiveX:master

cartant mentioned this pull request

Fix operators that fail the synchronous firehose test #5658

Closed

17 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet