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

observableOf and observableFrom instead of of and from #3514

Closed
benlesh opened this issue Apr 2, 2018 · 11 comments
Closed

observableOf and observableFrom instead of of and from #3514

benlesh opened this issue Apr 2, 2018 · 11 comments

Comments

@benlesh
Copy link
Member

benlesh commented Apr 2, 2018

Issues

  • People have strong feelings
  • IDEs like to color of in error
  • People that aren't used to RxJS don't know what of(1, 2, 3) means

Proposal

Rename of and from to observableOf and observableFrom.

Pros

  • It's less ambiguous and more explicit what code is doing
  • IDEs are happier about it
  • can make the change gradually without breaking everyone with an alias and a deprecation

Cons

  • It's WAY more verbose and makes code a bit more annoying to read.
  • It's another change to the library.
  • Most people using RxJS a lot in their apps know what of and from mean

Other thoughts

  • People can already to this with import aliases. `import { of as observableOf } from 'rxjs'; and the five extra characters there probably isn't going to kill them.
  • It might actually encourage the use of Arrays as return values (which is totally fine) inside of mergeMap et al: source$.pipe(mergeMap(() => of(1, 2, 3))) is the same as source$.pipe(mergeMap(() => [1, 2, 3])). The readability of this versus of is debatable.
  • observableFrom is probably the more annoying of the two changes, as it would be used more often to convert a promise ahead of other operators inside of flattening operators.
@kwonoj
Copy link
Member

kwonoj commented Apr 2, 2018

I'm indifferent with this one, but if we'are going to rename of, I'd vote to go for just instead - which most of Rx bindings (including .net's) are using but RxJS is a different one for those interface. It'll allow alignment between other bindings, instead of making name verbose like observableOf.

@c69
Copy link

c69 commented Apr 3, 2018

On our project, we already rename all such imports, e.g import { of as observableOf } from ....

@alex-okrushko
Copy link

The following works without any drawbacks:

import * as observable from 'rxjs';

observable.of(foo)

@Airblader
Copy link
Contributor

@alex-okrushko Importing like that will prevent tree-shaking, AFAIK. Also it requires you to prefix everything, not just the two functions in question.

@alex-okrushko
Copy link

@Airblader to take the guessing out of the equation here are the results:

import { Component, Input } from '@angular/core';
import * as observable from 'rxjs';
import {take} from 'rxjs/operators';

@Component({
  selector: 'hello',
  template: `<h1>Inteval at {{interval | async}}!</h1>`,
  styles: [`h1 { font-family: Lato; }`]
})
export class HelloComponent  {
  interval = observable.interval(1000).pipe(take(10));
}

Before I added the take operator and interval here how it looked after
ng build --prod --build-optimizer --source-map --eval-source-map

screen shot 2018-05-09 at 12 28 39 pm

There are some operators, but those are using by Angular itself.

Here is after I added interval to the above mentioned Component:

interval

As you can see even though import * as observable from 'rxjs'; was used only interval appeared in the optimized code.

I think this should resolve it.

@Airblader
Copy link
Contributor

I think this should resolve it.

Certainly, thanks :-) I do have more questions about it, but that's a bit off topic here. I'll research it on my own.

@r3h0
Copy link

r3h0 commented May 10, 2018

@alex-okrushko That's an interesting result. I was also under the same impression as @Airblader that doing import *... would prevent tree shaking.

I now see that Ben mentioned that this is possible in another issue.

My understanding is that modern tree shakers will pick this up.

I might start doing that myself now that I know it's OK. For those new to RxJS, like me, it's helpful being able to do rx. and have the IDE provide the functions in that module. I'll maybe do the same with operators, although it's a bit more onerous to have to prefix everything.

Seems like doing import * as rx from "rxjs" or some such would be a workaround for those having issues with of and from.

As to the names of and from being unclear about what they do, if you rename of and from don't you also have to rename things like fromEvent and the other functions that create Observables? This goes similarly for operators.

@r3h0
Copy link

r3h0 commented May 10, 2018

If you decide to rename of and from because of naming collisions with TypeScript keywords, please also consider doing the same with never, which you mentioned in this issue.

@benlesh
Copy link
Member Author

benlesh commented May 22, 2018

@r3h0. NEVER is a constant now, differing from TypeScript never in case.

@ajcrites
Copy link
Contributor

I am personally in favor of not removing operators (unless there is a collision), so I wouldn't want the rename. Doing import * as rx or import { of as rxOf } and the like are simple and acceptable alternatives that will keep the existing library more backwards compatible.

@shexrawa
Copy link

shexrawa commented Feb 7, 2021

@Airblader to take the guessing out of the equation here are the results:

import { Component, Input } from '@angular/core';
import * as observable from 'rxjs';
import {take} from 'rxjs/operators';

@Component({
  selector: 'hello',
  template: `<h1>Inteval at {{interval | async}}!</h1>`,
  styles: [`h1 { font-family: Lato; }`]
})
export class HelloComponent  {
  interval = observable.interval(1000).pipe(take(10));
}

Before I added the take operator and interval here how it looked after
ng build --prod --build-optimizer --source-map --eval-source-map

screen shot 2018-05-09 at 12 28 39 pm

There are some operators, but those are using by Angular itself.

Here is after I added interval to the above mentioned Component:

interval

As you can see even though import * as observable from 'rxjs'; was used only interval appeared in the optimized code.

I think this should resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants