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

Modules structure #1386

Closed
fdecampredon opened this issue Feb 24, 2016 · 8 comments
Closed

Modules structure #1386

fdecampredon opened this issue Feb 24, 2016 · 8 comments

Comments

@fdecampredon
Copy link

I'm starting to use RxJS next in my project instead of previous version, and I'm a bit confused about how module are exported.
For example the fact that this project export operators as named export instead of default export is a bit confusing, and it prevents some babel plugins to transform exports (ala lodash-babel-plugin).
Also I feel that having two separate function one static one for method just create complexity for nothing, could not we transform:

export function merge<T, R>(...observables: Array<Observable<any> | Scheduler | number>): Observable<R> {
  observables.unshift(this);
  return mergeStatic.apply(this, observables);
}

export function mergeStatic<T, R>(...observables: Array<Observable<any> | Scheduler | number>): Observable<R> {
.....
}

to something like :

export default function merge<T, R>(...observables: Array<Observable<any> | Scheduler | number>): Observable<R> {
  if (typeof this !== undefined) {
    observables.unshift(this);
  }
  return mergeStatic.apply(this, observables);
}
@kwonoj
Copy link
Member

kwonoj commented Feb 24, 2016

First, this issue #663 provides some background details why dropped default export but choose named export. But issue you mentioned is interesting, would you be able to share some snippets to replicate transform behavior?

Separate function for static / operator method, I personally do not think it causes great complexity especially there's ongoing progresses of reducing duplication like #1351 . Having different function was intended to reduce polymorphic behavior as much which doesn't show strong difference in this case though.

@fdecampredon
Copy link
Author

First, this issue #663 provides some background details why dropped default export but choose named export.

It seems to me that most of the issues of #663 are now more or less outdated, also we could have something like :

export function map() {

}
export default map;

which would satisfy everyone.

But issue you mentioned is interesting, would you be able to share some snippets to replicate transform behavior?

With 'babel-plugin-lodash'

import { map, filter } from 'lodash';
....

is transformed to

import map from 'lodash/map';
import filter from 'lodash/filter';
....

I would like to try doing the same with operators :

import { map, filter } from 'rxjs/operators';
....

transformed into

import map from 'rxjs/operators/map';
import filter from 'rxjs/operators/filters';
....

I'm trying to create a generic plugin for that purpose.
Sure I could provide an option that would allows to switch from import default to named import per module, but it just makes everything more complex.

Finally it seems to me that the logic behind es6 module is that when you export only one thing in a module, default export is the preferred way, it does not force the user to know which name he should import.
( and in case of catch or switch operator where the name of the function is in fact _catch and _switch it seems a lot more simple).

Separate function for static / operator method, I personally do not think it causes great complexity especially there's ongoing progresses of reducing duplication like #1351 . Having different function was intended to reduce polymorphic behavior as much which doesn't show strong difference in this case though.

I can understand why it has been done, but still having to import 2 different function depending of if you want to use them as plain function or as method feel like a poor developing experience.

@trxcllnt
Copy link
Member

@fdecampredon the biggest issue was interop between TS + ES6 + ES5-AMD + ES5-CJS (especially the last two). Since export default <fn> in ES6 isn't the same thing as module.exports = <fn> in ES5-CJS, anybody who wanted to use Rx in CJS was forced to write require('rxjs/operators/map').default, which was a nightmare. The decision was made to use named exports that since they work across all environments.

And for the instance vs. static methods, initially there was no distinction (as you can see here). I believe they've been separated to define more specific type signatures for the TS folks.

That said, I would love to support a plugin for Rx similar to babel-plugin-lodash so we can finally do away with the silly distinction between "core" and "kitchen-sink" features. @Blesh @mattpodwysocki

@fdecampredon
Copy link
Author

@fdecampredon the biggest issue was interop between TS + ES6 + ES5-AMD + ES5-CJS (especially the last two). Since export default in ES6 isn't the same thing as module.exports = in ES5-CJS, anybody who wanted to use Rx in CJS was forced to write require('rxjs/operators/map').default, which was a nightmare. The decision was made to use named exports that since they work across all environments.

Like I said we could export both default and named export and still have the same compatibility :

export function map() {

}
export default map;

Which is compiled by typescript to :

function map() {
}
exports.map = map;
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = map;

And for the instance vs. static methods, initially there was no distinction (as you can see here). I believe they've been separated to define more specific type signatures for the TS folks.

In the end it just makes thing harder to use for every non typescript user, which I guess is the majority.
Also typescript 2.0 will support both variadic types and this typing this could help to have proper signature with one single function ....

@david-driscoll
Copy link
Member

Generally speaking the main difference between the type signatures of static vs instance is the this context, and the scheduler.

It's not that it can't be done, just until now it hasn't been challenged to be done.

I personally don't care either way, until TypeScript gets :: I'll stick with augmentation to get my operators into place, vs using the operators themselves.

@fdecampredon
Copy link
Author

Would it be useful if I worked on a PR for example that implements the few change that I'm describing ?

@benlesh
Copy link
Member

benlesh commented Feb 21, 2017

closed as inactive.

@benlesh benlesh closed this as completed Feb 21, 2017
@lock
Copy link

lock bot commented Jun 6, 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 6, 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

5 participants