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.

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

Split creation functions from creation functions with Schedulers #3264

Closed
benlesh opened this issue Jan 26, 2018 · 7 comments
Closed

Split creation functions from creation functions with Schedulers #3264

benlesh opened this issue Jan 26, 2018 · 7 comments

Comments

@benlesh
Copy link
Member

benlesh commented Jan 26, 2018

Currently we have creation functions like so:

  • from(obj) and from(obj, scheduler)
  • range(a, b) and range(a, b, scheduler)
  • (... and many more)

Problem

Schedulers and scheduling adds a LOT of bloat to bundles and we can't currently avoid it.

If you look at the implementations of these you'll see an obvious pattern, especially in the ones I more recently created as "just functions":

function fromSomething<T>(something: Something<T>, scheduler?: IScheduler) {
  if (!scheduler) {
     return new Observable(/* a little bit of code */);
  } else {
    return new Observable<T>(subscriber => {
      /* OMG WHAT?!! A HUGE MOUNTAIN OF CODE AND A BUNCH OF TYPES
          WE NEED TO IMPORT FOR THIS SO WE CAN DO THINGS
          WITH SCHEDULERS, WHICH DON'T GET USED 95% OF THE TIME */
    });
  }
}

/* MORE STUFF DOWN HERE FOR SCHEDULING SUPPORT */
function dispatch() {
  /* you get the idea */
}

Proposal

  • Split creation methods into "normal" and "scheduled" flavors: from and fromScheduled, defer and deferScheduled, range and rangeScheduled etc.

If we do this, then the majority of RxJS usage (which doesn't involve schedulers) can avoid pulling in scheduler-related classes and scheduling-related logic.

That would make the above into:

95% use case

function fromSomething<T>(something: Something<T>) {
     return new Observable(/* a little bit of code */);
}

and else where

5% use case

function fromSomethingScheduled<T>(something: Something<T>, scheduler: IScheduler) {
    return new Observable<T>(subscriber => {
      /* OMG WHAT?!! A HUGE MOUNTAIN OF CODE AND A BUNCH OF TYPES
          WE NEED TO IMPORT FOR THIS SO WE CAN DO THINGS
          WITH SCHEDULERS, WHICH DON'T GET USED 95% OF THE TIME */
    });
}

/* MORE STUFF DOWN HERE FOR SCHEDULING SUPPORT */
function dispatch() {
  /* you get the idea */
}

Risks

It will be a breaking change for people using schedulers today. Easy to fix, but annoying.

@kwonoj
Copy link
Member

kwonoj commented Jan 26, 2018

Do we have guesstimate of user numbers uses scheduler when create? I do obviously, and all of my test cases are. I'm feeling it might be confusing number of api surface even if separated out as own export 🤔

@benlesh
Copy link
Member Author

benlesh commented Jan 26, 2018

@kwonoj I can do a search of google projects to get a pretty good ballpark number.

@kwonoj
Copy link
Member

kwonoj commented Jan 26, 2018

Plus if fns are separated, what's recommended pattern to inject testscheduler for observables?

@benlesh
Copy link
Member Author

benlesh commented Jan 26, 2018

Plus if fns are separated, what's recommended pattern to inject testscheduler for observables?

That wouldn't change... from(whatever) with no Scheduler would never need a TestScheduler.

The change proposed above would obviously not include things like timer and interval.

@cartant
Copy link
Collaborator

cartant commented Jan 26, 2018

Is the proposal for a public rxjs/schedule import location? Is that really needed if there aren't name conflicts? Introducing another location seems to be at odds with #3262.

Otherwise, sounds good to me.

@benlesh
Copy link
Member Author

benlesh commented Jan 26, 2018

@cartant you're right, we can probably keep them all in the same spot. 🤷‍♂️

(I changed the OP to reflect this)

@benlesh
Copy link
Member Author

benlesh commented Oct 2, 2019

Done.

@benlesh benlesh closed this as completed Oct 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants