Stop publishing rxjs-es. Move ES6 build to output .mjs files alongside the CJS .js files.

[benlesh]

In speaking with @robwormald the Angular folks are forced to swap between rxjs and rxjs-es during their build to enable proper tree-shaking. This is probably less than ideal. If all of the es6 files were under the same directories as .mjs files, then it would be as easy as configuring the bundler to look at that file extension to use ES6 modules.

This would mean that the rxjs-es build would likely not have a use anymore.

cc/ @IgorMinar @jeffbcross @bradlygreen

[jayphelps]

I'm not big on jumping on the .mjs bandwagon so dang soon, particularly since it is so controversial..it's far from decided.

That said, if we did publish .mjs files alone alongside the .js CJS files, it wouldn't hurt anyone but ourselves down the road if the .mjs thing dies but people are still depending on rxjs supporting it. So if we're OK with committing to support it even if .mjs dies (at least for a transitional period), then I'm not otherwise against it.

Also--I do not think we should stop publishing rxjs-es regardless because there will definitely be people who don't subscribe to the .mjs pitch but still want to consume the raw ES6 module version via standard .js extensions.

[robwormald]

should be clear - i'm not advocating at all the .mjs bandwagon - in fact we're currently on the jsnext:main bandwagon (via rollup) - which ideally has es6 sources distributed in a subfolder of the cjs distro and a field in the package.json pointing at it's entry point.

Sigh.

[jayphelps]

@robwormald Great clarification, I didn't realize that. oy vey module systems...

I'm way more OK with including the source as ES6 modules w/ .js extensions inside the normal rxjs CJS package, but inside a dir like es6/*.js and then pointing jsnext:main to it.

esm might be a better dir name since I believe we want to transpile al the syntax to ES5 except the module syntax.

[jayphelps]

Just to make things more interesting, TypeScript does not currently support targeting ES5 but keeping ES6 module syntax. e.g. tsc --target es5 --module es6 doesn't work.

Open issue: microsoft/TypeScript#6319

Sooooo...we could output entirely ES6 code, but then you must transpile all ES6 syntax, not just the module syntax. Is that OK? AFAIK rollup only supports the module syntax, not all of ES6 so you'd need an additional transpilation step in between......

[jayphelps]

hahaha actually, we could build esm/*.js files by building rxjs in TS -> ES6 then pipe that into babel to output ES5 w/ ES6 module syntax...gawd help us.

[robwormald]

we (currently) figure that just outputting full on ES6 code there is probably fine for the moment. Note that latest versions of TS allow .js input with --allowJs so you can use a single tool across the chain - i'm currently doing TS -> ES6 -> rollup -> ES5 via tsc.

[robwormald]

We're doing the esm subdir thing. Makes as much (or as little...) sense as anything right now

[benlesh]

So, @robwormald and @jayphelps ... are we currently, then, proposing to moving to build everything out into one package?

Basically, what we currently publish to rxjs (which is the CJS files), but also with an esm folder? (and perhaps a ts folder, I guess?)

In turn, we'd have one UMD output, and would no longer produce a special AMD bulid?

[robwormald]

that all seems reasonable to me. we don't need a ts subdirectory, as long as the d.ts files are distributed alongside. so it should look like

/Observable.js //commonjs
/Observable.d.ts
/esm/Observable.js //es6
/esm/Observable.d.ts

[kylecordes]

Over on some much less complex libraries than RxJS, we have been doing the same thing as @robwormald explained above, with excellent results. It "just works" and can be consumed without special tooling effort both as commonJS ES5 and ES6 with ES6 modules. Lacking any better plan (and with similar hesitation on the MJS thing) I think this would be an excellent step forward for RxJS.

[IgorMinar]

+1

there are no great options out there right now, but the jsnext:main as
used by angular is the thing that works with some caveats. one of them
should be researched by rxjs - deep imports will break in the esm mode.
This means that we need a story for add/operator/foo imports because
these won't work out of the box.

keep in mind that there are two kinds of path resolutions going on:

1/ typescript does a resolution when looking for type declarations stored
in d.ts files - typescript currently doesn't consider jsnext:main and will
grab cjs d.ts files. This is likely ok because the d.ts files are currently
the same for cjs and esm. For this mode the deep imports are ok.
2/ bundler (rollup, webpack, system builder) resolves all import paths and
all of the support jsnext:main entry point. For this mode deep imports
won't work out of the box and special care needs to be taken to make them
work.

we (angular) opted to not support deep imports except for a few very
special cases (e.g. cases when always defaulting to cjs is ok - like when
providing e2e test utilities).

On Sun, May 29, 2016 at 9:26 AM Kyle Cordes notifications@github.com
wrote:

Over on some much less complex libraries than RxJS, we have been doing the
same thing as @robwormald https://github.com/robwormald explained
above, with excellent results. It "just works" and can be consumed without
special tooling effort both as commonJS ES5 and ES6 with ES6 modules.
Lacking any better plan (and with similar hesitation on the MJS thing) I
think this would be an excellent step forward for RxJS.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1671 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AANM6E_A2nxc2iiVf9nchOM74uM3kYBCks5qGb4ngaJpZM4ITMsm
.

[benlesh]

@IgorMinar, interesting. I'd there a write up as to why deep imports don't work with bundlers?

[robwormald]

The tldr is that while its easy to remap an entry point file via something like jsnext:main, its not quite as trivial to remap a whole folder, and you end up having to do more configuration in your bundling tools. Not a huge deal but it adds some mental overhead.

[benlesh]

@robwormald @IgorMinar so what's actionable out of this issue?

Seems like:

1. Change ES6 build to build to an es or esm/ folder. (Probably just targeting ES5 features with ES6 modules if possible)
2. Add jsnext:main to package.json

Is that it?

Related: I also want to stop building the AMD stuff in favor of UMD, and rename the UMD output file to just Rx.js rather than Rx.umd.js

[IgorMinar]

1. We need to figure out how to make adding operators work.

On Thu, Jun 2, 2016 at 10:22 AM Ben Lesh notifications@github.com wrote:

@robwormald https://github.com/robwormald @IgorMinar
https://github.com/IgorMinar so what's actionable out of this issue?

Seems like:

1. Change ES6 build to build to an es or esm/ folder. (Probably just
   targeting ES5 features with ES6 modules if possible)
2. Add jsnext:main to package.json

Is that it?

Related: I also want to stop building the AMD stuff in favor of UMD, and
rename the UMD output file to just Rx.js rather than Rx.umd.js

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1671 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AANM6DSJKC0Xj-UuigBCguiJY5FSVrQ9ks5qHxE_gaJpZM4ITMsm
.

[kylecordes]

As an expedient way to get the current mechanism easily importable via ES6, one easyish answer is to mechanically create a top level module in the RxJS package which exports a bunch of separate add functions, one for each operator. I think there is a way to shape this file correctly that the packaging mechanism only picks up the specific things you want. So instead of this:

import "rxjs/add/operator/map";

You do this:

import {addMap} from "rxjs/addOperators";

?

[IgorMinar]

that wouldn't work for importing type information for that operator.

On Thu, Jun 2, 2016 at 11:50 AM Kyle Cordes notifications@github.com
wrote:

As an expedient way to get the current mechanism easily importable via
ES6, one easyish answer is to mechanically create a top level module in the
RxJS package which exports a bunch of separate add functions, one for each
operator. I think there is a way to shape this file correctly that the
packaging mechanism only picks up the specific things you want. So instead
of this:

import "rxjs/add/operator/map";

You do this:

import {addMap} from "rxjs/addOperators";

?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1671 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AANM6CPlo7hl46G3ws3BG3zfWZ03zxIIks5qHyX2gaJpZM4ITMsm
.

[benlesh]

@robwormald could we codegen remappings for people at build time and distribute it with the package?

[kylecordes]

importing type information

Ah, I had forgotten the very clever type set up. Okay, here are more ideas:

import "rxjs/add-operator-map";

It's not all that bad to look at, and it fits in the ability to have just one "/" in the path.

import "@rxjs/add-operator/map";

By this scheme we get two "/" and still have support for node resolution with esnext. Obviously this implies shipping rxjs as a set of packages, like Angular itself.

Either of the these things could be generated rather than and coded.

This latter model is a bit of a blast the past, it wasn't that many months ago when this was all in scoped packages. I don't know the motivation for moving it back out to a top-level package, but now that Angular itself is in the scope package certainly everyone is going to find them acceptably familiar and all the tooling is going to have to work well enough with them anyway.

[jayphelps]

btw TypeScript is very shortly going to support tsc --target es5 --module es6 (i.e. transpile everything except output ES modules) microsoft/TypeScript#9042

[benlesh]

Revisiting this... @IgorMinar, can you clarify this statement?

For this mode deep imports won't work out of the box and special care needs to be taken to make them work.

Why will deep imports not work? if import 'rxjs/Observable' looks at /esm/Observable.js why would import 'rxjs/add/operator/map' not look at /esm/add/operator/map.js?

[robwormald]

Why will deep imports not work? if import 'rxjs/Observable' looks at /esm/Observable.js why would import 'rxjs/add/operator/map' not look at /esm/add/operator/map.js

Because, apparently, that's just the way it is, see: rollup/rollup-plugin-node-resolve#44

What we need is agreement (LO-freaking-L) on a jsnext:dir

[robwormald]

FYI, we've implemented this structure in Angular2:

index.js (ES5 + ES Modules)
bundles/core.umd.js (UMD + ES5)

package.json:

• main : bundles/core.js
• module: index.js

So far, so good. This allows commonJS users to simply require('rxjs'), and browser users to deep import w/o worrying about paths.

[danbucholtz]

The .js files in rxjs-es contain es2015 code (let, const, arrow functions, etc). The es2015 module declarations are great for tree shaking but the code needs to be transpiled to es5 to be used by most browsers/apps.

Our use case was tree shaking out unused rxjs stuff from an Angular 2 library (Ionic Framework).

We are using the module field in our bundler to use the es2015 modules for Angular, and we would love to see rxjs do the same. 😃

Thanks so much,
Dan

[benlesh]

Okay, this is really the last issue blocking release, AFAICT... I feel we have a few options:

1. Stop publishing the ES Module build entirely prior to release
   • Remove this build before 5.0 release
   • bikeshed what we want to do for ES2015 module support, but still give everyone the .ts files with which they can build whatever processes they want
   • Once we decide on a strategy, we implement it and it's only a minor version change (v5.1 for example)
2. Publish .mjs files on rxjs package and cross our fingers a breaking change isn't required later.
3. Continue publishing rxjs-es and cross our fingers and cross our fingers a breaking change isn't required later.

Both 2 and 3 still require the change that we should be outputting ES5 code with ES2015 modules.

Looking at the npm stats, rxjs-es is not even close to as broadly used as rxjs. We're talking a couple of orders of magnitude. It's clearly not a high value for our users, but seems to be of some value.

[benlesh]

@robwormald wouldn't the plan you outlined above prevent "tree-shaking" and/or encourage usage that would forgo the advantages of modularity in RxJS 5?

[danbucholtz]

The Ionic team is following suit with what Angular is doing and encouraging others to do the same. What @robwormald is suggesting is the way to go IMO. I am currently producing documentation for library makers on how to get their lib into this state as we have seen a lot of popular Angular libraries still using main and commonjs without including the module field and esm code in the dist.

@Blesh, no it would not because bundlers will read the module option before the main option, and module points to esm code.

Thanks,
Dan

[jayphelps]

After discussing this further IRL, we're not entirely sure of the actual benefits of tree shaking for RxJS itself. RxJS is almost entirely operator based. So let me elaborate.

If you import everything a la import Rx from 'rxjs';

All the operators are added to either Observable.XXX or Observable.prototype.XXX. So right away all the operators are "unshakable" because they are found on objects and you cannot use static analysis to guarantee they are not used somewhere. You could place restrictions on certain JS features to make that guarantee, but to my knowledge Rollup, Webpack, etc do not attempt or plan to attempt anything like this--instead shaking modules by detecting used export bindings along with verifying there are no internal references to that binding. In this case, there are indeed internal references to the operator code, so even though they are exported they cannot be tossed because they could in fact be used.

import Rx from 'rxjs';
const operatorChoice = prompt('which operator?');
Observable[operatorChoice]();

It's similar to how in webpack you can't do require(prompt('which module?')) for much more obvious reasons.

If you import operators individually to prototype import 'rxjs/add/operator/map';

There is nothing to shake. You are only importing operators you intend use, which they will be added to the Observable prototype as a side effect. Even if you didn't end up using the ones you import, again static analysis cannot confirm this with certainty. (without restricting JS, which I believe is outside the scope of current implementations)

If you import operators individually and apply them as needed import { map } 'rxjs/operator/map';

Here you could in theory do some shaking in the sense that if someone imports an operator but in fact never references it in that module at all. I'm not sure if Rollup/webpack do this. My guess would be no because it provides little practical value. If you're not using something, don't import it. Very often people have lint rules that detect unused references/imports because it can be a sign of bugs.

Things other than operators

We next thought about things that aren't operators: the different Subjects, Schedulers, etc.

Nearly all of them are imported and consumed by some operator. e.g. publishBehavior = BehaviorSubject + ConnectableObservable. Though certainly there will be some comparatively tiny things that could be shaken off like TestScheduler. An argument could be made that small amount of code is worth it, so I'm not outright saying it's not.

---

In summary, we're skeptical of the practical benefits. Can anyone see something we missed? Maybe we're missing the point. Is there something other than tree shaking that this enables? This may still be worth doing simply so we don't have to explain why not to everyone haha even if it provides modest benefit.

[thelgevold]

I randomly saw this issue and got curious...

In the Angular 2 docs we use the commonjs plugin to convert rx-js commonjs to esm during bundling.
https://github.com/rollup/rollup-plugin-commonjs

Is there a practical difference between doing that and targeting released esm source like es-rxjs directly?

If there is no practical difference, isn't this easier than publishing two versions of the source?

I remember there was an extra step integrating the es-rxjs code with rollup but not too bad at the end of the day though.

export default {
  entry: '../../toh-6/ts/app/main-aot.js',
  dest: 'dist/build.js', // output a single application bundle
  sourceMap: true,
  sourceMapFile: 'dist/build.js.map',
  format: 'iife',
  plugins: [
      nodeResolve({jsnext: true, module: true}),
      commonjs({
        include: ['node_modules/rxjs/**']// convert the rx-js commonjs code here
      }),
      uglify()
  ]
}

[Rich-Harris]

@jayphelps I think your assessment is correct (at least based on my relatively slim knowledge of Rx) that tree-shaking will provide minimal benefits as things stand.

You could place restrictions on certain JS features to make that guarantee, but to my knowledge Rollup, Webpack, etc do not attempt or plan to attempt anything like this

We do have vague plans to improve the tree-shaking to the level that Rollup is able to remove unused class methods etc, but it's all a bit blue sky at the moment with no actual working code. If it does see the light of day, it won't be for a while.

My own inclination if rxjs-es is being killed off would be to ship deep imports in an esm subfolder (ES5 + modules), so that people still have a route to use ESM and deep imports without inconveniencing the majority – they might be a couple orders of magnitude smaller right now but I suspect once Webpack 2 ships there's going to be a surge of interest in this sort of thing as people come to understand the benefits. I'd definitely steer clear of .mjs!

[jayphelps]

@Rich-Harris thanks for the insight! 👍

---

@thelgevold I don't quite follow what you're suggesting. We would need to publish two versions of the source, both in ES5 but differing module formats CJS/ESM. I think you are prolly referring to npm packages e.g. rxjs vs. rxjs-es which is a valid point. If the most popular ESM bundlers use the module field we could indeed ship both versions within the same package. I think this is a likely outcome, but it's not without negatives. Somewhere around doubling the size of our npm package isn't trivial, particularly because some notable Electron users of Rx have already expressed issue with our existing package sizes; Slack being one, Nuclide the other. Doubling it obviously is a move in the opposite direction. If they started using a bundler, it would make this moot but that's obviously not a choice we should try and force on them--I have indeed suggested it.

We may still do it, just wanted to bring up some concerns.

[thelgevold]

@jayphelps I was thinking that the commonjs -> esm conversion in the bundler (via plugin) would remove the need for publishing esm at all. Wouldn't it be enough to only publish the commonjs source like in the current rxjs npm package?

I might be missing something though :-)

[jayphelps]

@thelgevold I think ESM -> CJS -> ESM is a lossy conversion in theory; which is why true tree shaking is only done with ESM and that plugin provides ways for you to "tell it" how to handle ambiguous cases. In practice it might almost certainly does not matter for Rx's usage of exports, I'm not sure. Either way I'm inclined to distribute lossless (true) ESM builds so that at the very least people stop bugging us about it 😆. It is indeed the latest fad.

[thelgevold]

@jayphelps That makes sense. Thanks for the explenation

[jeffbcross]

Somewhere around doubling the size of our npm package isn't trivial, particularly because some notable Electron users of Rx have already expressed issue with our existing package sizes;

Why? Are they shipping the full contents of node_modules folder with their applications?

[jayphelps]

@jeffbcross #YOLO I've seen this repeatedly with Electron users, not just those using RxJS

[benlesh]

At this point, I'm leaning in favor of just ceasing to publish rxjs-es ahead of the v5.0 release. Given that:

1. Rollup can only tree-shake about 1% of RxJS 5, so it's not really a great way to use this library.
2. We can bike shed how we want to publish es2015 modules all we want and then add it later without it being a breaking change.
3. Using rxjs-es is confusing and is of limited popularity anyhow.

[dbazile]

Would someone be able to regenerate the docs for http://reactivex.io/rxjs/manual/installation.html#es6-via-npm? As of 2017-08-22, it still recommends npm install rxjs-es.

I just installed rxjs-es (thinking it was still in favor) and stumbled upon this thread and #2218 after TypeScript raised a fit about file name casing. 😅

[lock]

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.