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

Rename of to just #3747

Closed
benlesh opened this issue May 25, 2018 · 31 comments
Closed

Rename of to just #3747

benlesh opened this issue May 25, 2018 · 31 comments

Comments

@benlesh
Copy link
Member

benlesh commented May 25, 2018

After speaking with @kwonoj and others, we think it would be best to rename of to just.

  1. It's still terse
  2. There's precedent (it was just in RxJS 4 and under, and in other Rx dialects)
  3. It's less confusing to read than of
  4. IDEs don't color code just like they do of

Thoughts? Questions? Concerns?

The process here would be to:

  1. deprecate of, but leave it
  2. Add just (of would alias just)
@benlesh
Copy link
Member Author

benlesh commented May 25, 2018

Related #3514

@claudiordgz
Copy link
Contributor

Oh no of is only in the bindings of PHP! 😨

I got no preference, both of and just are short enough, just is short enough to implement and long enough to make it a little bit more readable... observableOf seems overkill, wouldn't the rest of the observable functions have to follow a similar pattern? Consistency and all that?

@alex-okrushko
Copy link

I assume there was some prior discussion to move from just to of. Do you happen to know where that is?

@kwonoj
Copy link
Member

kwonoj commented May 25, 2018

As attached #3514 have history around.

@alexcastillo
Copy link

I feel “just” is not very meaningful and provides no context. Is “observableOf” not an option? It is very descriptive and leaves less room for confusion.

@benlesh
Copy link
Member Author

benlesh commented May 26, 2018

observableOf is really verbose, that's my only objection. It's unfortunate that "observable" is such a long word haha.

@mattwilson1024
Copy link

My team always aliases of to observableOf in the import, so I'm pleased to see that get suggested. It's a little more verbose, granted, but it reads nicely and makes things a lot more obvious in my opinion.

@wesleygrimes
Copy link

What about ‘oof’? Short for “observableOf”...

@leifwells
Copy link

of makes sense to me. But not all of these "action words" make sense, so I guess it doesn't need to make sense.

So, if you're going to do something, option 2 would be my preference, along with clear guidance.

@cartant
Copy link
Collaborator

cartant commented May 26, 2018

@wesleygrimes I was going to suggest - facetiously - that o10f could be used, but oof is better!

Seriously, though, I'm fine with just.

@Meligy
Copy link

Meligy commented May 26, 2018

If I have to choose, I'd keep of. Because just doesn't sound good in code that's all about composition.

If we have to change of (worst problem might be IDE colouring. It's also not a very clear word, but alternatives will probably have the same problem to be short), then maybe a totally new word. I thought of with but it also comes with its own problems.

For now, just of is fine.

@alex-okrushko
Copy link

@kwonoj #3514 has the recent history (which I participated in), I was asking to the history when just was dropped in favor of of. Is that available? I'm curious what the thinking was at the time that lead to such decision.

@kwonoj
Copy link
Member

kwonoj commented May 26, 2018

@alex-okrushko there was no filed issue in repo around those afaik.

@benlesh
Copy link
Member Author

benlesh commented May 26, 2018

Basically, the current of is just the implementation of the static Observable.of per the tc39 proposal.

RxJS 4 had Observable.of, return and just that all did the same thing. In version 5 we reduced it to the one that was required for the tc39 proposal.

@cartant
Copy link
Collaborator

cartant commented May 26, 2018

... the current of is just the implementation of the static Observable.of per the tc39 proposal.

But with support for schedulers added.

Now I don't know whether I prefer of or just. 🤔

@SirMaxxx
Copy link

Would not 'from' be appropriate?
'just' implies some sort of filter (just the first, just three etc.) Whereas what it is doing is providing an observable from something else.

@moniuch
Copy link
Contributor

moniuch commented May 26, 2018

@SirMaxxx from is another thing

@moniuch
Copy link
Contributor

moniuch commented May 26, 2018

To me it was easier to learn (and grasp the semantic difference vs from) with of. As weird as it may sound, just doesn't convey anything to me.

It's less confusing to read than of

Disagree. It is a generalization

IDEs don't color code just like they do of

Never had problems with syntax coloring, but when you deprecate of and when I stick with that (as I certainly will), I might start having strikethrough over of.

Add just (of would alias just)

Nope, opposite: just would alias of.

There's precedent

Exactly this is why I am against it. Please don't change api every other while. This change is not needed. It is yet another api change within a very short period of time and the value of it is questionable.

Not sure if a custom operator for creation can be achieved as easily as custom pipeable ones. If so - why not use that approach, if not - let's rather have a way to create custom operator for creation and then everyone will be able to have their on just, oof, iwannahavemyownof, a, etc.

So not really a good change in my eyes. 👎

@sandangel
Copy link

sandangel commented May 26, 2018

I want please()

@topaxi
Copy link

topaxi commented May 26, 2018

I like of as it mirrors Array.of(), standalone of seems a bit ambiguous though, as with any other observable creator function or operator.

const values = of('a', 'b').pipe(map(value => value.toUpperCase()))

Doesn't tell me whether I'm using observables or not. I'm not sure how relevant this is though, sometimes I just imagine how the current state would work if we had these functions for ES collections too.

Best case would probably be if usage like this could be treeshakable:

import * as Observable from 'rxjs';
import * as rx from 'rxjs/operators';

Observable.of()
Observable.from()
new Observable.Observable() // awkward
Observable.create()
Observable.EMPTY

const values = Observable.of('a', 'b').pipe(rx.map(value => value.toUpperCase()))

I also quite like the mentioned issue about observableOf and observableFrom, terse variants could be renamed with import { observableOf as of } from 'rxjs'.

@SirMaxxx
Copy link

@moniuch Although from is 'another thing' I still think it makes sense in this context.
Although maybe
'streamFrom' where elements of a collection will be emitted individually
'from' when the whole collection will be emitted.

The current difference between from and of is subtle - and I don't think will be aided by making it from and just (I know it used to be just, but I don't get why?)

But something like 'streamFrom' and 'From' emphasise the difference?

@emilio-martinez
Copy link

I think of makes more sense when you read it out like "create a stream of" vs "just create a stream".

Having some alignment with Rx implementations in other languages would make sense; not sure what the precedent is in that regard. However, ultimately aligning with Array.of() and Array.from() might carry more weight in JavaScriptland; people might find it more understandable if the same nomenclature is used.

@SanderElias
Copy link

I do like the current of, even with the syntax highlighting that might be a problem. I think just is just the wrong word ;) Also, I'll try to avoid the word 'just' completely in my explanations of tech stuff, as it sets people off. I know it's not really related that way, but given the choice, we should just avoid just.
If it must change, I prefer the oof that is mentioned above.

@sayax
Copy link

sayax commented May 26, 2018

Did you consider 'as'?

@paulpdaniels
Copy link
Contributor

If I can throw in another candidate into the ring only

@patrickschaffrath
Copy link

patrickschaffrath commented May 26, 2018

I like just, it reminds me of the Maybe Monad from Haskell. Makes sense to me.

@ajcrites
Copy link
Contributor

ajcrites commented May 26, 2018

I will defend of:

  1. It matches the TC39 proposal
  2. It functions similarly to Array.of
  3. I don't find it less confusing to read -- in fact I like that it gets syntax highlighted. You will generally only use it as of( with a parent vs. the keyword which will generally not have a paren. I also think it's unlikely to be used with / near the keyword, but I could be totally wrong about that.
  4. from has the same problem wrt IDE coloring and existence as a keyword.
  5. We don't like removing operators. This will ultimately require big changes for any code already depending on of that want to upgrade. This also has the potential of introducing something like yarn add rxjs-of-compat which would introduce more friction and dependencies -- especially for libraries with a peer dependency on an RxJS that may not have of but that use of.
  6. of is great for examples. Tons of existing examples will suddenly become deprecated and ultimately invalid. This introduces more friction for people learning RxJS which I think already has a fairly high barrier to entry as it is.

#WeLoveOf

Re: just, I automatically don't like it because it's new. It also confuses me a little bit -- like I suppose it's "just emit these values," but I don't have a personal basis for that since I didn't really start working with RxJS until v5. I kind of like only, but it seems to imply that it will emit a single value.

Other possible names could be: each, list, emit, immediate, now, values

However I'd prefer to keep of and from as they are.

@cartant
Copy link
Collaborator

cartant commented May 27, 2018

I've changed my mind. I think of should be left as it is.

Aligning RxJS with other Rx implementations has some appeal - as I've had some exposure to other implementations - but I doubt that's the situation for most RxJS users.

I think point number six in @ajcrites's defence of of is significant. Surely any gains from just being "less confusing to read than of" need to be offset against the confusion that will be effected by a large proportion of current examples being rendered out-of-date? Whether it is "less confusing" is subjective, anyway.

Neither of nor from are keywords, so if IDEs choose to colour them differently outside of for or import contexts, that's a problem with the IDEs. I don't think that's a compelling reason to effect a change to such fundamental functions. Their use is, after all, not an error.

@cartant
Copy link
Collaborator

cartant commented May 27, 2018

Whatever the outcome of this issue, I'll write some TSLint rules so that those that don't get the outcome they want might be less unhappy.

If of is deprecated, I'll write two rules:

  • one to replace the use of of with just;
  • and another to alias just as of (as an import binding) and use the alias (for those who steadfastly refuse to change).

If of is kept, I'll write a rule to:

  • alias of as just and use the alias.

@waitingsong
Copy link

waitingsong commented May 27, 2018

For me it seems rename from to ofrom (or any other) is more effective then of, cause of too many matched from such as import {...} from ... during global search.

@benlesh
Copy link
Member Author

benlesh commented Jun 1, 2018

Okay... so given the very mixed reaction to this issue, I think we're at a stand-off. In light of that, I'm closing the issue, and we'll just stick with the status quo.

of stays of for now. It's easy enough for people to alias it as just or observableOf or whatever they want. We can perhaps provide guidance in the docs for the time being.

@benlesh benlesh closed this as completed Jun 1, 2018
@ReactiveX ReactiveX locked as resolved and limited conversation to collaborators Jun 1, 2018
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