Rxjs: Angular/Closure users need an ESM distro of rxjs

[alexeagle]

See https://github.com/angular/angular/issues/8550#issuecomment-277498959
We are close to supporting Angular apps bundled by Closure compiler. It's way smaller than the alternatives and I expect a number of big apps / enterprises will want to evaluate this.

Currently the last blocker is that we need a custom build of RxJS to make this work. We also need one extra bit in there - we use @angular/tsc-wrapped (or just ngc) in place of tsc in order to work around a few closure limitations (no /index implicit imports, no export *, need complete JSDoc type annotations, etc).

I could send a PR but the bigger issue is the release engineering steps to support a second build, where to publish it (or just replace the current distro as a breaking change), etc. @jayphelps I know you were planning to revisit the topic of an ES6 distro after 5 final shipped. Can we discuss?

[jayphelps]

I'll add this as an agenda item for the next meeting. Do you want to join us? It's in 1 week, 2/13 at 10:30am PST over Hangouts

[alexeagle]

Sounds good thanks

On Mon, Feb 6, 2017 at 10:55 AM Jay Phelps [e-mail] wrote:

I'll add this as an agenda item for the next meeting. Do you want to join
us? It's in 1 week, 2/13 at 10:30am PST

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ReactiveX/rxjs/issues/2335#issuecomment-277776710,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5Iyf02-LFQP1TK_Wahz_Dcd4vhtglks5rZ2yCgaJpZM4L4cds
.

[alexeagle]

Hey @jayphelps I finally got time to verify this.

my repo now has three options for building rxjs (see the README.md: https://github.com/alexeagle/closure-compiler-angular-bundling#rxjs-distros )

It seems that all three are pretty much the same size. As you pointed out, since you don't use export * or /index imports, the tsickle step in @angular/tsc-wrapped was not required.

So I believe we just need an ES5+ESM distribution of RxJS to satisfy this use case.

[alexeagle]

Hey, any update @jayphelps ?

[jayphelps]

@alexeagle hey, @kwonoj will follow up with you

[kwonoj]

I think we can probably discuss this on next core team meeting. few questions on my end if you don't mind

It seems that all three are pretty much the same size.

: is this means whatever RxJS module you use (es6, cjs..) size is nearly same? means tree shaking is not so much effective?

ES5+ESM distribution of RxJS

: is this meaning physically ESM should be included in ES5(CJS) package? or it's ok with any other physical package published?

[alexeagle]

Right, I think the dead code elimination is not doing much, and in any of the packaging we only get the operators that are used.

Any package that uses ES modules should work for us. I assume es5 is easiest for you, since it's similar to what you already ship.

[alexeagle]

Hey, any update? It would be nice to have an ESM release of rxjs to go with Angular 4 final

[trxcllnt]

@alexeagle pretty sure we can output any combo of es5/es6 and es-modules you need. Would ES6 + ESM be best for Angular 4? @benlesh I'm happy to tackle this tonight. For publishing this version, should we resurrect the rxjs-es module?

[kwonoj]

For publishing this version, should we resurrect the rxjs-es module?

: I'd +1 for this.

[kylecordes]

@trxcllnt I request that it not be packaged in a separate NPM package. But rather that it be packaged right in RxJS, pointed to by the "es2015" field, just like Angular. (I'm not really sure "es2015" is the best choice for this field though... if something different emerges as a standard, please follow it...)

[kwonoj]

@kylecordes is there specific reason not to publish as separate package? Your suggestion is let scoped pkg (@reactivex/rxjs) to have all dist?

[kylecordes]

@kwonoj sure, here are reasons:

• I would like to refer to it universally as just "rxjs/whatever", from the usage point of view the code should not care how it was packaged.
• I prefer not to pollute the namespace for every package with a combination of package, package-es, and package-some-future-format-here.

Regarding the scope packaging, certainly those seem to be the big thing right now. I have not been following rxjs all that closely, but if rxjs moves to "@reactivex/rxjs" that would seem quite defensible.

If that changes, I think still all of the different ways of bundling the code should live in the same NPM package. For example this done very nicely, look at Angular. They have been around the block repeatedly with different ways of packaging, and appear to have come up with a very nice well thought out combination.

[imcotton]

Isn't that pkg.module for? Supports by both Rollup and webpack v2.

[kwonoj]

@kylecordes While I can totally understand motivation, I'm also strongly opposed to use existing commonjs package rxjs to ship with es2015 codes along with. The usecases you've mentioned is pretty much already solved via our scoped package @reactivex/rxjs, that you won't hit both cases of how its packaged, also doesn't have separate pkg concern based on formats.

Above usecases works very well for users who targets browser and bundler for modules, but there are sufficient number of users who don't / can't fit for specific usecases such as users of Electron. Once if cjs ships ES modules, those users do not have any options of package to have CJS only but have to ship unnecessary codebase they aren't using.

[kwonoj]

So my proposal is pretty much as what RxJS did in beta phases.

• rxjs : CJS only
• @reactivex/rxjs: contains CJS, ES, (and global as well) for unified scoped package
• rxjs-es: ES only, anyone who want to opt in ES native modules

[kylecordes]

@imcotton pkg.module typically means "ES5 code in ES2015 modules". As far as I know, it's not yet an official NPM standard, but seems to be somewhat widely used. It is mentioned in the rollup docs: https://github.com/rollup/rollup/wiki/pkg.module

I am not aware of any standard that has emerged for "ES2015 code for ES2015 modules". So far I see Angular using "es2015" for this purpose. If rxjs follows along that will be two of them.

Depending on the build process and tooling one has in mind, either of these can be most well-suited.

@kwonoj What is the benefit in shipping three different packages to contain different module formats?

Here is a cost: application and library code would change between various different ways of importing:

import {Observable} from 'rxjs/Observable';

import {Observable} from 'rxjs-es/Observable';

import {Observable} from '@reactivex/rxjs/Observable';

... based on the build tooling that the project was going to use. I don't feel like application code should care at all what packaging style is preferred by a build process. So I guess the essence is: should the various different packagings be treated as just different styles of boxes that (for example) "Observable" arrives in? Or in application code should we think of rxjs/Observable and rxjs-es/Observable as different things to care about the differences between?

[kwonoj]

@kylecordes I'd say this is cost of there isn't standard ES native module spec in node.js packaging. While it 'does' cost consumer side, also there isn't way to make all possible consumer's need via single package. Again, your proposal of use-one-package-all via specific field is quite tailored way for angular 2 usecases. Consumer of angular 2 would be perfectly happy with that, but that doesn't work for other consumers like Electron, as I commented about. I'd rather open up way to opt in for each individual usecases. Once there is finalized spec arrives (probably .mjs proposal I assume?) we could end up discussions of how to publish native ES module though, we're not there yet so far I know.

It is notable thing in typical usecase for now you don't even need to consider what package to use - plain cjs rxjs is fit for most cases. This is specific cases for ng2 build with closure compiler, and other cases there is practically no difference to use cjs or native ES since implementation of RxJS doesn't provide benefit like ES module tree shaking. So most user just need to from 'rxjs/Observable';, and for ng2 only we could guide to use ES module instead, if needed.

[kylecordes]

@kwonoj Thank you, yes, I see. Although the machinery I use (webpack, rollup, Closure) can easily accommodate nascent not-yet-standards for including different bundle/module types in a single package, some other stuff out there (Electron) can't yet. Ouch.

It appears (it will be interesting to see what anyone from Angular says) that Angular "wants" (is it okay to anthropomorphize a computer program?) to live today in the future world where NPM packages routinely include multiple bundle/module types. Wanting it doesn't make it true though.

To fit with the world as it actually is, as I look at this I'm wondering if all of my Angular code, as well as possibly Angular itself, should say:

import {Observable} from '@reactivex/rxjs/Observable';
import '@reactivex/rxjs/add/observable/share';

etc., throughout. I think how you describe things above, implies this. It looks like a straightforward search-and-replace to get there.

[trxcllnt]

@kylecordes if you're using webpack, you can alias 'rxjs' to any package, e.g. 'rxjs-es' or '@reactivex/rxjs/dist/es6'. I'm not the biggest fan of relying on specific build tools, but react has set somewhat of a precedent here, and we don't have many options when it comes to es-reliant features -_-

[alexeagle]

@trxcllnt @kwonoj any chance to get a distro this week? Next week is ngConf and I'd like to be able to announce/document Angular + Closure Compiler. This is the last blocker.

Re. having both in one package, this is less important for me. Actually Angular depends on rxjs both in nodejs runtime (when we run Angular's compiler) and also in the browser runtime, where rxjs libraries get bundled into the application. Right now this means we would depend on both packages. Could you send me a snapshot of whatever you intend to publish, and I'll verify that it works with our tooling?

Thanks! (and @benlesh too)

[kwonoj]

@alexeagle sorry, I was having bad cold last week and couldn't followup issues. Though it's late, I'll try to come up with some PR within this week if schedule allows it.

[alexeagle]

Thanks @kwonoj I'm happy to help as well. But I suspect most of the work is the release engineering aspects (like updating some publishing instructions). Let me know what I can do...

[david-driscoll]

Why wouldn't we just look at something like package.module? It would mean that anyone using angular would have to reference rxjs-es where most of the docs strictly talk about rxjs. I feel like it would an extra level of confusion for users that's unnecessary.

It may also lead to compiler problems, where I've seen before where we get an error like Observable<T> is not assignable to Observable<T>. This is honestly my biggest fear. There may very well be a huge pain for anyone upgrading to the version of angular that consumes this new package. Since angular will move to rxjs-es, if the users don't also move over to rxjs-es the TypeScript compiler may not be very happy with that, as you'll have two modules, that have the same definition, but will emit errors anytime an observable is emitted. It could be part of a major change, but I can envision the hundreds of Stackoverflow questions coming in already.

I know it's extra files for electron apps, which does suck, however having different module names may be a very big problem for TypeScript consumers, and specificly the very heavy TypeScript Angular angular base. Is this a break we want to kick off? @alexeagle @benlesh

---

Original comment: https://github.com/ReactiveX/rxjs/pull/2547#issuecomment-293372853

[kwonoj]

As I've shared already in above, my main motivation to object @david-driscoll are

1. CJS module have unnecessary distro
   : For users who can't use bundler like Electron

2. package.module is not standard form of supporting ES module
   : For me this is important that there isn't anything defined standard way to ship ES native modules. afaik package.modules is also not part of node / npm's but only for specific bundlers like rollup or webpack. Yes, it is quite majority but that doesn't mean futureproof guaranteed, as well as opens lot of questions like i.e some another popular bundler comes out and asking other way around, or any other framework chooses complete different paths to support ES native modules (like early adoption of .mjs maybe? I can't imagine what it can be). Similar to our zones support cases, general approach for this library should be user / environment agnostic.

It is definitely well-fitted solution for ng users, majority of user have specific pipeline configurations but as a library, I don't see strong reason it should be suited in those way. Once node.js started support native ES modules all of this confusions can go away, but that's something we can't have it right now.

[david-driscoll]

@kwonoj I agree with you 100%.

• It's extra crap that not everyone needs ( 😿 )
• Until it's a standard we should definitely use caution. Still most of the popular loaders / compilers do support this. ( 😿😿😿 )

The problem I'm getting at, and the repro is attached.

• If anyone upgrades to use the future version of angular that uses rxjs-es, but doesn't update their code, they will be broken.
• If they consume a library that uses rxjs but doesn't have any angular bindings, they will be broken.

Mind the zip file size, I included a "fake" angular version here, but the demonstration is easy to show that it will break consumers who do not also update to the correct module name.

Updated: forgot to save the file after taking the screenshot 🐑
test.zip

[david-driscoll]

In addition if you want to offer a library that consumes rxjs, you now have to produce two versions one that consumes rxjs and rxjs-es. There is no good way around the problem that I can see. If someone can find one, I would be totally down with that.

[alexeagle]

@david-driscoll thanks for the repro. The reason this setup works for Angular apps today is that no one imports from rxjs-es, nor should they. User code, library code, and Angular all import types from rxjs and type-check fine. I see in your repo that @angular/http.d.ts has an artificial dependency on rxjs-es which is not the real situation.

We only swap in the files from rxjs-es when handing .js files to Closure Compiler for optimization. At this point, the .d.ts files in the rxjs-es distro are never read, and there is no problem.

I agree this could be a sharp edge for users, who should not try to import from the -es package. I will try another experiment to get closure to work with the existing commonJS package...

[david-driscoll]

As long as the dependency is on the same library I think we'd be okay, I was trying to emulate the case I know will break the compiler, which is bad for all the users (of which there are many! lol).

It still feels like we could cause an unwanted fork for all rxjs consumers, which I thought that was why we stopped publishing rxjs-es in the first place. If it's used as a background drop-in for something like closure compiler that works for me, but the moment a large library takes a public dependency on it, we may be in for a load of hurt. :boxing_glove:

[kylecordes]

I'm trying to get a good understanding on the reasoning of why the main rxjs package ought not contain alternate bundlings also. From reading above it sounds like the primary motivation here is that extra files would be dead weight for certain kinds of consumers, including for example Electron users.

Having worked with lots of NPM packages, I think it is common for them to contain extra files, extra bundles, beyond the minimum needed to function. But that could be an unusual experience, maybe there are many common packages used by Electron users which are already narrowed down to the minimum workable set of files.

(Having seen what I've seen in terms of lots of extra files, if I were putting together something with Electron I would plan on trying to run the code and libraries through Webpack or Rollup prior to bundling in Electron, to leave behind all the excess. But that that is not a built-in feature of Electron, I assume.)

[kwonoj]

@kylecordes if my comment above gives implication electron is important so I'd like to avoid CJS pkging, that's obviously my fault and apologize that I didn't mean it.

My take is basically in https://github.com/ReactiveX/rxjs/issues/2335#issuecomment-293379370 's 2 - while there isn't standard way to achieve this, all of propsed solution to ship ES modules in CJS is though it's supported by well-known mechanism that probably will exclude users who doesn't follow those practices.

For example, if node.js defines putting .mjs alongside of CJS as standard way of shipping ES module with CJS, even if it makes pkg size increase to electron user's I would vote for those. My approach for this is quite similar to support zone.js support, if there's some 3rd party support is being required and if it's possible, it should be framework, or library agnostic way, to able to support any possible further requirement comes in further. Currently suggested solution, pkg.module, I couldn't see viable way if it could be supported other than users of rollups or webpack at this moment.

I could be definitely wrong, and if there's gracefull way to support ES module pkgs along with CJS not dependent on specific bundler's config or etcs, I'll definitely vote for those regardless of if it takes more spaces for electron users cause via those generic way any consumer (like electron) could think of way to shape up modules by their own.

[kylecordes]

@kwonoj I here you on the thing about wishing there was a standard way of doing this. Me too.

I have not read the NPM specs, but I have been reading the contents of highly numerous NPM packages while working on projects that use lots of them. It appears to me that the community has chosen to interpret the specs as:

• A package must contain CommonJS files.
• The pkg.main must point to the CommonJS entry point file.
• It's okay for a package to contain arbitrarily many other things, including other module formats, as long as the CommonJS files don't inadvertently refer to them and cause trouble for CommonJS consumers.
• It's okay for packages that contain extra package fields that aren't standards yet, pointing to files in formats that aren't standards yet, with the understanding that the future standard might break these packages if it chooses the same package field name with a different meaning than such a pre-standard package used.

Of course this is just my interpretation, based on looking at how the tool seemed to be used. Such things can vary greatly from how the specs or tool authors had in mind.

[kwonoj]

@kylecordes that is pretty much concise explanation of custom pkg expansion, but it doesn't explain 'way to resolve to support various usecases' part. Following that means we could go with pkg.module, but that doesn't mean we can resolve some other requirement comes like please support other way 2 instead of pkg.module, part of my comment about 'agnostic support'. Unless there is either de facto way (like typings field in package.json, since practically there's only one way, typescript compiler to consume typings) or either real standard comes in, I'd still object with pkg.module approach.

I know this isn't something have concrete answer and I think I've shamelessly iterated my opinion too verbose, so I'll leave it up open for other people's opinion as well.

[alexeagle]

FYI I think Angular is satisfied with the 5.3.1 release, so you could close this issue. There's good discussion here but the Original Post is no longer relevant.

[tomkel]

@kwonoj package.module is in the draft spec for node-esm integration:

https://github.com/nodejs/node-eps/blob/master/002-es6-modules.md#51-determining-if-source-is-an-es-module

[lock[bot]]

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.