Move rxjs/add/* modules to a separate package for 6.0?

[benlesh]

Most of us are in agreement that we'd advise people use the pipeable operators instead of the prototype patching for a variety of reasons that have been outlined elsewhere.

Do we want to keep the rxjs/add/* modules in the same package as everything else, or do we want to have a separate package to support dot-chaining on NPM?

Or... more extreme... should we eliminate them? Maybe even wait for v 7 for that?

Either way, how could we best educate people of the caveats?

[benlesh]

@kwonoj @david-driscoll

[kwonoj]

This seems related with #2911, so in next major, dot chain operator syntax is no longer default thing? I think this discussion is really depends on those direction.

[benlesh]

Yeah, I suppose it's related for sure. Although it doesn't really have to be... you can definitely have dot-chaining without lift.

[kwonoj]

There should be long other discussions need to be done, But first I'd downvote to eliminating it completely. It's too radical even for major breaking.

[benlesh]

@kwonoj I think I agree. I don't think I'm down for completely eliminating it. At least not in the short to mid-term. But a separation might provide people better guidance and clarity.

[kwonoj]

yes, I'm sort of agreeing to separate packages, especially given size constraint we'd like to achieve in pkg. Either case it'll be huge breaking so through guide / migration process should be prepared.

[felixfbecker]

A separate package could possibly have very confusing bugs with node module resolution / node_modules structure where the Observable import may not resolve to the correct version (I've hit bugs like that and they are insanely hard to debug). It would also mean it's harder to upgrade to v6 (e.g. a Greenkeeper PR won't do it). I would vote for keeping them in the main package for now and deprecate them. Then they can be removed in a future major version when most people are using lettable operators.

[benlesh]

@felixfbecker the separate package would need to have peer-dependency requirements.

[wardpeet]

so talking to @ladyleet for a minute on slack I think I understand what you want to do.

My suggestion is to bring lerna which makes it possible to publish multiple packages from the root with the same version.

so we probably start with

• core (all of rxjs)
• websocket subject
• ajax subject

[debuggerpk]

I am undecided if this is the right place to ask this question, or StackOverflow might be best for this. So here i go.

My introductions to rxjs was through the excellent Angular framework and throughout every tutorial and in practice has been "import whatever you need from add/operator and then chain it. eg.

import { Observable } from 'rxjs/Observable';
import { empty } from 'rxjs/observable/empty';
import 'rxjs/add/operator/catch';
import 'rxjs/add/operator/map';
// Other rxjs import
import 'rxjs/add/observable/of';

Class SomeClass {
  @Effect
  search$ = this._action
    .ofType<search.SearchCountry>(search.SEARCH_COUNTRY)
    .map(action => {
      return action.payload;
    })
    .switchMap(query => {
      if (query === '') {
        return empty();
      }
      const nextSearch$ = this._action.ofType(search.SEARCH_COUNTRY).skip(1);

      return this._searchService
        .search(query)
        .do(query => console.log(query))
        .takeUntil(nextSearch$)
        .map((result: RestResponseInterface) => {
          return new search.SearchCountryComplete(result);
        });
    })
    .catch(error => {
      return Observable.of(new search.SearchError('Undocumented API Error'));
    });
}

As explained in lettable documentaion, and If I am reading this right, chaining is discouraged as way forward, and pipe is the recommended implementation. The same implementation should be written as follows

import { tap, take, takeUntil, skip, map, switchMap } from 'rxjs/operators'; // Import statements have changed

search$ = this._action
	.pipe(
		ofType<search.SearchCountry>(search.SEARCH_COUNTRY),
		map(action => action.payload),
		switchMap(query => {
			if (query === '') {
				return empty();
			}

			const nextSearch$ = this._action.ofType(search.SEARCH_COUNTRY).skip(1);

			return this._searchService
				.search(query)
				.pipe(tap(qyery => console.log(query)), takeUntil(nextSearch$), map((result: RestResponseInterface) => new search.SearchCountryComplete(result)));
		})
	)
	.catch(error => {
		return Observable.of(new search.SearchError('Undocumented API Error'));
	});

Because when i read the source, especially after https://github.com/ReactiveX/rxjs/blob/master/src/Rx.ts#L41 its seems that chaining is perfectly acceptable, however this proposal is discussing adding rxjs/add/* as a seperate package.

My question is, how will this effect chaining in future?

[benlesh]

This is done. We effectively did this with rxjs-compat.