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

Additional rx* prefixed exports #3508

Closed
bisubus opened this issue Mar 31, 2018 · 5 comments
Closed

Additional rx* prefixed exports #3508

bisubus opened this issue Mar 31, 2018 · 5 comments

Comments

@bisubus
Copy link

bisubus commented Mar 31, 2018

Currently pipeable operator and observable creation exports lack a namespace and pollute a module where they are used with very succinct names that can collide with other imports and user variables.

The suggestion is to provide additional set of prefixed exports to the package. They won't provide breaking changes, won't significantly increase package footprint (especially if tree-shaking is in use) and won't increase output size (especially if minification is in use).

There are several important concerns that could be addressed this way. They primarily apply to observable methods/factories and operators, but other exports are affected, too (Symbol, Observable):

  • Increasingly complex API. Due to collisions between reserved words and package exports, same things have been available under different names (weird tap->do->tap change).

  • Ambiguity leads to a lack of context. In a module that is dedicated to observables, it's easy to spot that RxJS is in use and follow which variable names belong to RxJS API and which are not. In most real-life scripts, it becomes significantly harder to read and develop RxJS code without constantly checking import list or IDE hints; also results in name collisions.

  • Name collisions. The burden of resolving naming conflicts in imports is placed on a developer. It's easy to unintentionally mix them up or end up with inconsistent naming.

    • Observable factories vs. operators (merge, zip, etc.)
    • Observable factories and operators vs. other FP APIs like Lodash and ponyfill imports (every, find, debounce... plenty of them).
    • vs. globals (principally window, Symbol, potentially Observable)
    • vs. common user variables (count, pipe, delay, etc.)
    • never operator vs. TypeScript never (doesn't affect anything, just confusing)
  • Auto-imports. Export names are ambiguous, they should be picked by hand on IDE auto-import, and it's impossible for IDE to provide auto-imports automatically (for a piece of code that is pasted).

  • Single rxjs entry point. No confusing variety of imports like rxjs/operators (a potential cause for a known issue).

All of these concerns can be addressed by adding prefixes to exports:

  • Operators: zip -> rxZip, catchError -> rxCatch (canonical names can be used for operators that collide with reserved words)
  • Observable factories : zip -> rxCreateZip/rxStaticZip/???
  • Others: Symbol -> RxSymbol
@roblav96
Copy link

roblav96 commented Apr 1, 2018

@bisubus Couldn't agree more. This lib is already an organizational deterrent :/

@benlesh
Copy link
Member

benlesh commented Apr 1, 2018

Please use the template provided when submitting the issue. I'm not sure what version you're talking about.

Observable factories vs. operators (merge, zip, etc.)

The operator versions of these are being removed.

never operator vs. TypeScript never

This is only a concern for TypeScript users, but I see your point, probably worth a separate issue, along with of and observableOf etc.

prefixing things with rx...

You can all do that on your own if you which

import { zip as rxZip } from 'rxjs'

// or even

import * as rx from 'rxjs/operators';

// use rx.map, rx.zip etc. My understanding is that modern tree shakers will pick this up.

I'm going to close this one for now.

@benlesh benlesh closed this as completed Apr 1, 2018
@bisubus
Copy link
Author

bisubus commented Apr 2, 2018

@benlesh I didn't use issue template because this is proposal/feature request, not bug report. It affects current RxJS version, 6.0.0-rc.0. I can add a template if it's the only problem.

The operator versions of these are being removed.

It's good to know, but the rest of the points are applicable:

The burden of resolving naming conflicts in imports is placed on a developer.

Auto-imports. Export names are ambiguous, they should be picked by hand on IDE auto-import

@roblav96
Copy link

roblav96 commented Apr 2, 2018

@bisubus Do something like this:

import * as ops from 'rxjs/operator'

@lock
Copy link

lock bot commented Jun 5, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 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

3 participants