angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
94.88k stars 24.76k forks source link

UMD bundle cannot be used for RxJS #9359

Closed arpit-agarwal closed 7 years ago

arpit-agarwal commented 7 years ago

I'm submitting a ... (check one with "x")

[x] bug report
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior I updated system-config.ts file to download umd file for rxjs I.e rxjs\bundles\Rx.umd.js. This worked. But I still see in dev tools a lot of individual JS being load like rxjs\observer.js

Expected/desired behavior When using UMD for rxjs it should not download individual files

Reproduction of the problem See this plunker when you riun in full screen mode observe the files being loaded in dev tools. http://plnkr.co/edit/TvjW2YK3NVJ7sDb7cHV4?p=preview

No community response on question: http://stackoverflow.com/questions/37881825/using-rxjs-umd-bundles

What is the expected behavior? When using UMD for rxjs it should not download individual files

What is the motivation / use case for changing the behavior? To reduce load request when angular app starting up

Please tell us about your environment:

dragGH102 commented 7 years ago

+1. I had the same issue

arpit-agarwal commented 7 years ago

There are direct imports of Rx sub module in @anguard/http which may lead to this issue. If @angular/http start using rxjs from umd module then this could be resolved.

IgorMinar commented 7 years ago

@robwormald this might a bigger issue than what I thought and is caused by deep imports into rxjs which won't work with umd. System.register format would handle this well, but that has other issues.

pavandv commented 7 years ago

One solution I came up with is that loading Rx.umd.js through script tag and removing all the entries related to RxJS from system-config. This could resolve your issue.

jalkanen commented 7 years ago

@pavandv That solution does not seem to work; systemjs still attempts to load the individual script files.

IgorMinar commented 7 years ago

@pavandv @jalkanen you'd have to access rxjs via window.Rx then, right?

mlc-mlapis commented 7 years ago

Is there something new related to the problem with using RxJS bundle?

gkgithub commented 7 years ago

Is there any workaround until the issue is fixed. I tried loading through script and delete all rxjs entries in systemjs file. Still, systemjs tries to download the individual rxjs operator files. Its impacting the initial load performance

arpit-agarwal commented 7 years ago

You should use individual operators import (deep import). This will reduce the requests to 70-80 ( depending upon how heavily you use RxJS) from 275 by bundle like import

robinkedia commented 7 years ago

Hi - When can we expect a fix on this?

rpaschoal commented 7 years ago

@robinkedia,

It has a milestone related to RC5 upcoming release.

fabricioferreira commented 7 years ago

I don't think it is worth to use the umd bundle anyway. It seems Rxjs folks have broken down the components into smaller pieces, then you need to explicitly reference what you want to use. This have reduced the amount of files being downloaded a lot from previous versions.

At my company I switched to a different approach, where I copy all the .js files from the npm folder into a vendor folder using a gulp task:

const source = './node_modules/rxjs/**/*.js'; const dest = './wwwroot/vendor/rxjs'; module.exports.dep = function (gulp, plugins) { return function () { return plugins.del(dest); }; }; module.exports.task = function (gulp, plugins) { return function () { return gulp.src(source) .pipe(gulp.dest(dest)); }; };

And then map it using system.js.config: var map = { ... 'rxjs': './vendor/rxjs' ... }

At first I didn't like the solution, but later it proved to be acceptable.

awerlang commented 7 years ago

I don't think it is worth to use the umd bundle anyway. It seems Rxjs folks have broken down the components into smaller pieces, then you need to explicitly reference what you want to use. This have reduced the amount of files being downloaded a lot from previous versions.

Agreed. Loading individual files are only a problem in development mode with SystemJS loader. For production we bundle them into a single file for fast loading. But even if we get this to work (Rx UMD) we still have yet hundreds of files to deal with (on-the-fly typescript). So I believe the definitive answer for all of this is migration to webpack and angular-cli (I hope so).

TLDR: rxjs UMD is not helpful

robinkedia commented 7 years ago

Please help provide a interim solution if milestone has changed from RC5 to 2.0.1. Appreciate your support. Thanks!

stefanocke commented 7 years ago

As a workaround, I use rxjs SystemJS bundle in a script tag. It seems to work. No individual rxjs files are loaded. Are there any disadvantages with this approach (as workaround until umd can be used)?

mlc-mlapis commented 7 years ago

Yes, it looks good. I tried Rx.min.js from 'bundles' directory and no problems.

damiandennis commented 7 years ago

The including via script tag only works if you are not importing rxjs yourself.

damiandennis commented 7 years ago

There seems to be a problem with the new router v3 and rxjs, used to have below in "path" and it used to work:

 "rxjs/*": "node_modules/rxjs/bundles/Rx.umd.min.js"
damiandennis commented 7 years ago

related somehow to "of" and "ArrayObservable" just not sure why....

mlc-mlapis commented 7 years ago

What kind of problem you mean? We removed rxjs from system.js config completely and loading it only through the script tag. We use router V3 and no problems appeared.

mlc-mlapis commented 7 years ago

Hmm, "of" and "ArrayObservable", thanks. We will look.

damiandennis commented 7 years ago

bundling rxjs myself using systemjs-builder mostly fixed my issue. still get ~ 40 request though not really sure why. Here is how I bundled using gulp and systemjs-builder

gulp.task('ng-bundle-rxjs', function(done) {

    var builder = new Builder('./', './systemjs.config.js');
    builder
        .bundle([
            'node_modules/rxjs/add/**/*.js',
            'node_modules/rxjs/observable/**/*.js',
            'node_modules/rxjs/operator/**/*.js',
            'node_modules/rxjs/scheduler/**/*.js',
            'node_modules/rxjs/symbol/**/*.js',
            'node_modules/rxjs/util/**/*.js',
            'node_modules/rxjs/*.js'
        ], 'bundles/rxjs.min.js', {
            minify: true,
            sourceMaps: true,
            mangle: false
        })
        .then(function() {
            console.log('Build complete');
            done();
        })
        .catch(function(err) {
            console.log('Build error');
            console.log(err);
            done();
        });
});

and in systemjs.config.js

bundles: [
    "bundles/rxjs.min.js": ["rxjs/*"]
]
karlhaas commented 7 years ago

@damiandennis I took your approach and it works well as temporary solution. I think you should replace utils by util.

damiandennis commented 7 years ago

@karlhaas thanks that reduced another ~ 20 requests

ricardojbertolin commented 7 years ago

@damiandennis thank you for the proposed solution. I was able to bundling rxjs by myself, however, couldn't configure systemjs properly.

I mean, despite of using the bundle for rxjs, the angular modules (http, router, etc) still trying to load the individual rxjs libs.

Maybe there is something I'm forgetting? Thank you

animator013 commented 7 years ago

@damiandennis I can confirm that new Router v3.0.0-rc.1 breaks the path solution

"rxjs/*": "node_modules/rxjs/bundles/Rx.umd.min.js"

I get: TypeError: Cannot read property 'apply' of undefined

JanNorden commented 7 years ago

@mlc-mlapis When loading via a script tag, how do you get the typings right when you use rxjs types (such as Subject<> or Observable<>) in your own code?

mlc-mlapis commented 7 years ago

In a web browser it does not play any role. Typing is a factor for TS editor, compiler, ... I suppose that you use static compilation and not dynamic right in a browser.

Delagen commented 7 years ago

I make own webpack build of RxJS and angular umd bundles that works together.

damiandennis commented 7 years ago

This removes all but one request, Wildcard does not work it seems.

bundles: [
    "bundles/rxjs.min.js": [
        "rxjs/*",
        "rxjs/operator/*",
        "rxjs/observable/*",
        "rxjs/add/operator/*",
        "rxjs/add/observable/*",
        "rxjs/util/*"
    ]
]
Delagen commented 7 years ago

@animayor013 this cause a rxjs/operator/* imports that not resolvable in this rule

robinkedia commented 7 years ago

Before RC.6 I was using rxjs/bundles/rx.min.js in index.html to load 1 single file. With RC6 upgrade I do not see it any more. I just see rx.umd.js. If i use that I get bunch of 404 error. Even though i have suppressed rxjs in systemjs, it still attempts to load. It was working for me in .rc5

bahodirk commented 7 years ago

@karlhaas could you please share your solution? facing the same issue, can't find a way to make it run. Thank you.

karlhaas commented 7 years ago

@bahodirk I'm using the solution of @damiandennis . We had to change imports from 'rxjs' to 'rxjs/Rx' in our project.

robinkedia commented 7 years ago

Best is switch to webpack

On Monday, 5 September 2016, karlhaas notifications@github.com wrote:

@bahodirk https://github.com/bahodirk I'm using the solution of @damiandennis https://github.com/damiandennis . We had to change imports from 'rxjs' to 'rxjs/Rx' in our project.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/angular/issues/9359#issuecomment-244744673, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ6mG8ykN0MQrPmZFtePaAFrjvny1KXOks5qnBaNgaJpZM4I5Yvv .

Cheers!

damiandennis commented 7 years ago

@robinkedia If angular2's default quickstart example switched to webpack I would be happy to do so but since their examples are in systemjs it has been easier to fix breaking changes (have been using since beta).

woppa684 commented 7 years ago

Hey wait a minute, do I read between the lines that I can not use the new router without switching my entire build environment to webpack?

First I had to switch from the "script tag solution" to the "path solution" because we're using RxJS stuff in our code now. When I now wanted to start with the new router I immediately got:

core.umd.js:5995 EXCEPTION: Uncaught (in promise): TypeError: Cannot read property 'apply' of undefined

After reading all of this I still don't see a working solution with making use of the RxJS umd bundle right?

damiandennis commented 7 years ago

@woppa684 just build your own version of rxjs using systemjs builder as shown above.

woppa684 commented 7 years ago

Hmm, ok, thanks! Never used systemjs-builder but I'll see if I can find out how it works. Hope I don't have to change our build/deployment system everytime a new version of something comes out.

Within my company (Canon) we are stuck to Visual Studio for development, that's why I was very pleased when Angular2 was announced to be in TypeScript. Since our entire build is built around MSBuild I had to create numerous MSBuild targets to actually distibrute a working web application. Using "thirdparty" components like Angular and RxJS as bundles really helped (since we can not use npm, gulp etc). With RxJS not delivering a bundle anymore I have to see how I can fix this again...

Since Visual Studio automatically uses as build task for ts files I also don't see how I am going to use the new AoT compilation by the way. Ah well, yet another challenge I guess! :)

woppa684 commented 7 years ago

For the latest beta (12) of rxjs they removed the UMD bundle again by the way ...

kylecordes commented 7 years ago

@woppa684 Visual Studio works very well for A2 development - we train people using that regularly. However, it doesn't have all the things it needs on its own. You will end up needing, 99% likely, to run the needed Node-based tooling inside your Visual Studio environment. New Visual Studio versions make this increasingly easy.

(Using A2 with no Node involved in the build tooling at all - that could be tough.)

woppa684 commented 7 years ago

@kylecordes Yep, that's what we do ... we need to support systems in the field for 10 years after the latest sale so we have a lot of maintenance trees where we need to keep the build tooling as it was at that time. Therefore all thirdparty stuff we use is checked in in TFS (either binary or by source). We DO however have some node tooling available, but also this tooling is strictly versioned and kept with the trees in maintenance. Also, our build servers are not allowed to access the internet so then we would need to have local repos anyway ...

For example npm itself, typescript, karma, jasmine etc are considered development environment, however angular and rx are considered thirdparty libraries. Therefore the former are versioned by installers we create ourselves (to be installed on the development pc's of the engineers) and the latter is checked in in TFS together woth our code in the thirdparty section.

We also disallow engineers to create their own build configuration or build workflow, therefore we don't have files like package.json, tsconfig.json, karma.js etc in our project folders. For karma and webpack we have a generic one in our build tooling that has to work for all projects.

I'm pretty sure we should change this system sometime of course but changing stuff like this in a really large company takes a while ... For now we are very happy with A2 and we've created a pretty nice proto for the new UI of our printers, but having to find out with each release of either A2, zone.js or rxjs how the bundles have changed and how our build/bundling system has to change to fix it again is very, very tedious ... :)

Long story short, if anyone knows how to configure SystemJS to load everything I need from the Rx.min.js bundle provided with RxJS 5.0.0-beta.12 I'd be very happy, if not I will start investigating how to change our build/deployment system to overcome this :)

Huck commented 7 years ago

having the same (or similar) issue. been using rxjs through the systemjs config up to RC5 and it was working fine. In RC5, had to change for a script tag in index.html to include Rx.min.js. now that I upgraded to RC7 (and now 2.0.0), the index.html include doesn't work anymore. I reverted to the systemjs config file with:

paths: { 'rxjs/*': 'node/rxjs/Rx.min.js', },

but now I get errors in the router:

core.umd.min.js:21 Error: Uncaught (in promise): TypeError: Cannot read property 'call' of undefined at l (zone.min.js:1) at l (zone.min.js:1) at eval (zone.min.js:1) at e.invokeTask (zone.min.js:1) at Object.onInvokeTask (core.umd.min.js:29) at e.invokeTask (zone.min.js:1) at n.runTask (zone.min.js:1) at a (zone.min.js:1) at XMLHttpRequest.invoke (zone.min.js:1)ErrorHandler.handleError @ core.umd.min.js:21next @ core.umd.min.js:29generatorOrNext.object.schedulerFn @ [...] zone.min.js:1 Unhandled Promise rejection: Cannot read property 'call' of undefined ; Zone: angular ; Task: Promise.then ; Value: TypeError: Cannot read property 'call' of undefined(…) TypeError: Cannot read property 'call' of undefined at ApplyRedirects.expandSegmentGroup (http://localhost:5000/node/angular2/router/router.umd.min.js:20:8956) at ApplyRedirects.apply (http://localhost:5000/node/angular2/router/router.umd.min.js:20:7499) at applyRedirects (http://localhost:5000/node/angular2/router/router.umd.min.js:6:7976) at eval (http://localhost:5000/node/angular2/router/router.umd.min.js:20:29638) at new e (http://localhost:5000/node/zone/zone.min.js:1:18127) at Router.runNavigate (http://localhost:5000/node/angular2/router/router.umd.min.js:20:29443) at eval (http://localhost:5000/node/angular2/router/router.umd.min.js:20:28999) at e.invoke (http://localhost:5000/node/zone/zone.min.js:1:15913) at Object.onInvoke (http://localhost:5000/node/angular2/core/core.umd.min.js:29:16609) at e.invoke (http://localhost:5000/node/zone/zone.min.js:1:15864)

this is completely blocking us and is a serious problem.

due to our development model, we do not want to change for webpack any time soon. Systemjs with transpiling in the browser is perfect for us in development. in prod, we simply change to the compiled js. Squeezing every single bit of performance is not an issue for us internally.

thanks

edit: just replaced the Rx.min.js bundle with the full package, pointing to Rx.js instead and it works now, so the problems is definitely caused by Rx.min.js bundle. Current solution is really not ideal, because it adds almost 50 calls to the load time. how can we use the bundle in 2.0.0?

unlight commented 7 years ago

To fix this issue we need to generate Rx SystemJS bundle by ourselfs

const Builder = require("systemjs-builder");
// SystemJS build options.
var options = {
    normalize: true,
    runtime: false,
    sourceMaps: true,
    sourceMapContents: true,
    minify: false,
    mangle: false
};
var builder = new Builder('./');
builder.config({
    paths: {
        "n:*": "node_modules/*",
        "rxjs/*": "node_modules/rxjs/*.js",
    },
    map: {
        "rxjs": "n:rxjs",
    },
    packages: {
        "rxjs": {main: "Rx.js", defaultExtension: "js"},
    }
});

builder.bundle('rxjs', 'node_modules/.tmp/Rx.js', options),

This will generate bundle ready to consume by SystemJS.

Inject <script src="node_modules/.tmp/Rx.js"></script> to html

So, any rxjs/* import statements (e.g import rxjs/Observable) will not cause 50+ requests by SystemJS.

**Important**! In your `systemjs.config` should be nothing related to `rxjs`, if it will
For example:
```js
map = {
     "rxjs": "n:rxjs",
};

It will cause again 50+ requests...

Huck commented 7 years ago

@unlight

your solution works perfectly. thank you so much!

ricardojbertolin commented 7 years ago

After testing other solutions, @unlight your solution works definitely fine. thanks!!

subsite commented 7 years ago

Thank you @unlight it works great! I experimented further and added minification to the script:

var compressor = require('node-minify');

...

builder.bundle('rxjs', 'node_modules/.tmp/Rx.js', options)
    .then(function() {

    new compressor.minify({
        type: 'uglifyjs',
        fileIn: 'node_modules/.tmp/Rx.js',
        fileOut: 'node_modules/.tmp/Rx.min.js',
        callback: function(err, min){
            if (err) {
                console.log(err);
            }
        }
    });
});

The other compressors of node-minify (gcc, yui) threw errors, but uglifyjs works. Size went from 766K to 394K.

unlight commented 7 years ago

@subsite systemjs-builder has option minify and related to minification mangle. See https://github.com/systemjs/builder#minification--source-maps

subsite commented 7 years ago

@unlight Sorry, didn't read carefully enough. And thanks, tested builder's minify option and it works too, no need to complicate things.

ghost commented 7 years ago

@unlight , I will probably never meet you but you are my new best friend. Thanks to you I'm successfully bundling RxJS and it has dramatically reduced the number of HTTP requests. I still can't believe how excruciating it is to bundle, minify, and deploy Angular 2 apps from Visual Studio 2015, but your post helped me take a big step forward. Thanks again.