Rxjs: Rename do back to tap (or atleast re add alias)

Created on 19 Sep 2016  路  11Comments  路  Source: ReactiveX/rxjs

Choosing a reserved word just seems like it's asking for problems.

I can't for the life of me figure out how to get do to work with function bind syntax.

image

import { _do } from 'rxjs/operator/do';
Seems to work but the underscore makes me a little cautious.

discussion

Most helpful comment

I believe this is an exhaustive list:

  • catch
  • switch
  • do
  • finally
  • let
  • throw

I think we need to remove the recommendation around using bind operator because I think it is too prescriptive considering it's still stage-0 and somewhat controversial among TC39. We can certainly still show it as an option, but we should clarify and not suggest it as the "best" solution. People take what we say as gospel without knowing the costs.

All 11 comments

Look serious/legit.

This isn't specific to do, we have numerous operators which are reserved words in this context.

We're going to try to be more aggressive in releasing v5 final asap, so the ship has most likely sailed for v5 on this--but we can discuss solutions for v6.

In the future I'd personally like to see us move completely away from reserved words, instead of aliasing. This will be tough to bikeshed since most (or all?) of these are inherited from other Rx implementations so changing them causes us to diverge even further from the common "reactive extensions" operator names. But we have unique usage and requirements around JS so I think it's still appropriate to do what's right for JS.

There's so many names that coincide with reserved words. Observable.of to begin with. :(

@staltz of isn't a reserved word 馃槃 (reserved words)

@BLamy remember that you can easily alias imports

import { _do as tap } from 'rxjs/operator/do';

Ah, I assumed it was, as you can do nowadays for (var key of obj)

The only two operators which conflict with reserved words I could find that were let and do.

Aliasing was a simple workaround but is using function bind still the preferred approach? I was only using it because it's labeled as the recommended approach.

I expected the following 2 statements to be equivalent:

Observable.of(['foo']).do(console.log.bind(console)) 
Observable.of(['foo'])::do(::console.log) 

I believe this is an exhaustive list:

  • catch
  • switch
  • do
  • finally
  • let
  • throw

I think we need to remove the recommendation around using bind operator because I think it is too prescriptive considering it's still stage-0 and somewhat controversial among TC39. We can certainly still show it as an option, but we should clarify and not suggest it as the "best" solution. People take what we say as gospel without knowing the costs.

I think we need to remove the recommendation around using bind operator because I think it is too prescriptive considering it's still stage-0 and somewhat controversial among TC39. We can certainly still show it as an option, but we should clarify and not suggest it as the "best" solution. People take what we say as gospel without knowing the costs.

^^^ This seems actionable.

catch
switch
do
finally
let
throw

Renaming these would make migrations more painful, and is perhaps out-of-scope for v5.0? Particularly painful would be catch and finally, which are idioms from Promise. It seems like this is more of an issue for people pulling in the operators one-by-one and using them. At this point, I'm more in favor of recommending people use the operator patching modules (rxjs/add/operator/do, for example), as I've found them to be most beneficial to apps I've worked on.

@blesh removing that recommendation was done in #1962 馃憤

Regarding the reserved word issue, I think it's more of an issue for library authors like ng2, redux-observable, etc who need to consume operators in their code but don't want to attach them to the prototype because it would cause confusion and bugs with people depending on certain operators behind available in one release (because of implementation details) and then later it gets removed because it isn't need internally.

e.g. if ng2 uses .switch() in the code base but then one day refactors to no longer need it, if it was added to Observable.switch etc it could break user code who were depending on this--even though they shouldn't, it could have just been an accident.

I don't feel this is a blocking decision for v5, but we can discuss collectively at our rxjs meeting next week.

We did this with pipe operator.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

benlesh picture benlesh  路  3Comments

samherrmann picture samherrmann  路  3Comments

haf picture haf  路  3Comments

peterbakonyi05 picture peterbakonyi05  路  4Comments

dooreelko picture dooreelko  路  3Comments