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

Advanced production build #864

Closed
mgechev opened this Issue May 12, 2016 · 81 comments

Comments

Projects
None yet
@mgechev
Owner

mgechev commented May 12, 2016

Currently our production build is quite basic and produces only two bundles:

  • shims
  • libs + app

Which means that we're not taking advantage of:

  • lazy loading
  • offline template compilation
  • tree-shaking

Proposal

Incrementally improve the production build by:

Step 1

  • - Include offline template precompilation
  • - Introduce tree-shaking with rollup.js / Google Closure Compiler

Step 2

  • - Introduce bundling by bounded context (lazy-loading boundary) strategy
@hmagrini

This comment has been minimized.

Show comment
Hide comment
@hmagrini

hmagrini May 12, 2016

Would Critical Path CSS be something interesting to generate as well?

hmagrini commented May 12, 2016

Would Critical Path CSS be something interesting to generate as well?

@phiphou

This comment has been minimized.

Show comment
Hide comment
@phiphou

phiphou May 12, 2016

As I said in #860 , application (only the code you/we wrote), vendors (angular2, rxjs and potential additional vendors like jquery, lodash...), and polyfills (reflect-metadata, zone, es6-shim,...) life-cycles aren't the same most of time. Keeping them in separate bundles seems to me a good practice. I'll be interested in knowing your thoughts about this point

I also agree lazy loading would be a great feature. It would obviously augment the amount of generated bundles, but I don't see any major reason not to working this way.

phiphou commented May 12, 2016

As I said in #860 , application (only the code you/we wrote), vendors (angular2, rxjs and potential additional vendors like jquery, lodash...), and polyfills (reflect-metadata, zone, es6-shim,...) life-cycles aren't the same most of time. Keeping them in separate bundles seems to me a good practice. I'll be interested in knowing your thoughts about this point

I also agree lazy loading would be a great feature. It would obviously augment the amount of generated bundles, but I don't see any major reason not to working this way.

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous May 12, 2016

Contributor

Lazy loading and hot loading are amazing. We could create a pattern ... I though, when it was changed, that components that started with + in it's directory would be lazy loaded only when needed. I like this pattern, what do you think?

Contributor

Bigous commented May 12, 2016

Lazy loading and hot loading are amazing. We could create a pattern ... I though, when it was changed, that components that started with + in it's directory would be lazy loaded only when needed. I like this pattern, what do you think?

@jlberrocal

This comment has been minimized.

Show comment
Hide comment
@jlberrocal

jlberrocal Jun 16, 2016

@Bigous i'm using the + for indicate that is a lazy loaded component, for development seems to work fine, but for production there is still only 2 JS files, one with the shims and a big one with the vendors and the app itself

jlberrocal commented Jun 16, 2016

@Bigous i'm using the + for indicate that is a lazy loaded component, for development seems to work fine, but for production there is still only 2 JS files, one with the shims and a big one with the vendors and the app itself

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous Jun 16, 2016

Contributor

Yes thats why I think we could create a name pattern for that. Bundle the components in other files, like AngularClass did with the webpack starter kit would be a nice one and would resolve the step 2. Unfortunately I've got almost no time during the past 3 months to help with the project, but the idea is still very valid.

Contributor

Bigous commented Jun 16, 2016

Yes thats why I think we could create a name pattern for that. Bundle the components in other files, like AngularClass did with the webpack starter kit would be a nice one and would resolve the step 2. Unfortunately I've got almost no time during the past 3 months to help with the project, but the idea is still very valid.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Jun 27, 2016

Owner

Here's a blog post I wrote about an experiment I did around the production build efficiency.

A working "Hello world!" example with precompilation + tree-shaking, can be found here.

Owner

mgechev commented Jun 27, 2016

Here's a blog post I wrote about an experiment I did around the production build efficiency.

A working "Hello world!" example with precompilation + tree-shaking, can be found here.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Jul 4, 2016

Owner

In addition - I'll be playing with the production build to introduce:

  • Brotli
  • Google Clojure Compiler

These tools should decrease the bundle size to around 20k.

Owner

mgechev commented Jul 4, 2016

In addition - I'll be playing with the production build to introduce:

  • Brotli
  • Google Clojure Compiler

These tools should decrease the bundle size to around 20k.

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous Jul 6, 2016

Contributor

20k footprint for ng2 is unbelievable! Awesome... and congratulations for the great article!

Contributor

Bigous commented Jul 6, 2016

20k footprint for ng2 is unbelievable! Awesome... and congratulations for the great article!

@vyakymenko

This comment has been minimized.

Show comment
Hide comment
@vyakymenko

vyakymenko Jul 6, 2016

Contributor

@mgechev , I'm not sure about Google Clojure Compiler, because there are so many real testing information about GCC vs UglyfyJS2.

https://www.peterbe.com/plog/advanced-closure-compiler-vs-uglifyjs2

But, it's a good point to test it with advanced flag.

Contributor

vyakymenko commented Jul 6, 2016

@mgechev , I'm not sure about Google Clojure Compiler, because there are so many real testing information about GCC vs UglyfyJS2.

https://www.peterbe.com/plog/advanced-closure-compiler-vs-uglifyjs2

But, it's a good point to test it with advanced flag.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Jul 15, 2016

Owner

Update: It is still too early to use offline compilation. The platform directives need to be explicitly configured and also I had issues making it work when ngModel directive is used.

In general, my plan is to implement a task called build.prod.exp which is going to be an experimental production build which includes everything from above.

Owner

mgechev commented Jul 15, 2016

Update: It is still too early to use offline compilation. The platform directives need to be explicitly configured and also I had issues making it work when ngModel directive is used.

In general, my plan is to implement a task called build.prod.exp which is going to be an experimental production build which includes everything from above.

@SekibOmazic

This comment has been minimized.

Show comment
Hide comment
@SekibOmazic

SekibOmazic Jul 15, 2016

@mgechev When could we expect tree-shaking with rollup.js? Any timeline on this?

SekibOmazic commented Jul 15, 2016

@mgechev When could we expect tree-shaking with rollup.js? Any timeline on this?

@mgechev

This comment has been minimized.

Show comment
Hide comment
@SekibOmazic

This comment has been minimized.

Show comment
Hide comment
@SekibOmazic

SekibOmazic Jul 16, 2016

@mgechev cool! Hey did you just add rollup.js?

SekibOmazic commented Jul 16, 2016

@mgechev cool! Hey did you just add rollup.js?

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Jul 16, 2016

Owner

Yes only rollup. There are some rxjs resolution issues that need to be addressed + refactoring, before being able to merge in master.

Owner

mgechev commented Jul 16, 2016

Yes only rollup. There are some rxjs resolution issues that need to be addressed + refactoring, before being able to merge in master.

@SekibOmazic

This comment has been minimized.

Show comment
Hide comment
@SekibOmazic

SekibOmazic Jul 16, 2016

@mgechev running gulp build.prod.exp creates a tmp/app.js of 1.6M. It doesn't look really "squeezed". Am I missing something?

SekibOmazic commented Jul 16, 2016

@mgechev running gulp build.prod.exp creates a tmp/app.js of 1.6M. It doesn't look really "squeezed". Am I missing something?

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Jul 17, 2016

Owner

There's no minification and compression of the code, as I already said the work in this branch is Work In Progress.

Owner

mgechev commented Jul 17, 2016

There's no minification and compression of the code, as I already said the work in this branch is Work In Progress.

@ZuSe

This comment has been minimized.

Show comment
Hide comment
@ZuSe

ZuSe Jul 22, 2016

Contributor

I also would like to see support for cloud foundry/heroku deployment.
Right now a lot of unnecessary files is shipped.

Contributor

ZuSe commented Jul 22, 2016

I also would like to see support for cloud foundry/heroku deployment.
Right now a lot of unnecessary files is shipped.

@Perezmarc

This comment has been minimized.

Show comment
Hide comment
@Perezmarc

Perezmarc Aug 1, 2016

Are you supporting gzipped build at the moment? my app.js file is 2.5mb and my project is not that huge. On production it is 1.6 s to load for the first time...

Perezmarc commented Aug 1, 2016

Are you supporting gzipped build at the moment? my app.js file is 2.5mb and my project is not that huge. On production it is 1.6 s to load for the first time...

@vyakymenko

This comment has been minimized.

Show comment
Hide comment
@vyakymenko

vyakymenko Aug 1, 2016

Contributor

@Perezmarc , gzipped build o_O, what do you mean when you talking about gzipped build ?

Gzip is using on server-side And you configure level of gzip compression as you want.

Contributor

vyakymenko commented Aug 1, 2016

@Perezmarc , gzipped build o_O, what do you mean when you talking about gzipped build ?

Gzip is using on server-side And you configure level of gzip compression as you want.

@Perezmarc

This comment has been minimized.

Show comment
Hide comment
@Perezmarc

Perezmarc Aug 1, 2016

sorry for my ignorance, looked at the post http://blog.mgechev.com/2016/06/26/tree-shaking-angular2-production-build-rollup-javascript/ and as build.prod currently generates a minified code, I thought that the next step for reducing the load time was gzipping it. :S I can't generate then a build to deploy it to S3 gzipped? In order to gzip it, what should i do?

Perezmarc commented Aug 1, 2016

sorry for my ignorance, looked at the post http://blog.mgechev.com/2016/06/26/tree-shaking-angular2-production-build-rollup-javascript/ and as build.prod currently generates a minified code, I thought that the next step for reducing the load time was gzipping it. :S I can't generate then a build to deploy it to S3 gzipped? In order to gzip it, what should i do?

@vyakymenko

This comment has been minimized.

Show comment
Hide comment
@vyakymenko

vyakymenko Aug 1, 2016

Contributor

@Perezmarc , for example, if you using NginX for representing your application, you can look for my example for configuring gzip compression here.

But, I'm still confusing, what do you mean about deploy gzipped code. I think you need more documentation and literature about how it's work.

Contributor

vyakymenko commented Aug 1, 2016

@Perezmarc , for example, if you using NginX for representing your application, you can look for my example for configuring gzip compression here.

But, I'm still confusing, what do you mean about deploy gzipped code. I think you need more documentation and literature about how it's work.

@Perezmarc

This comment has been minimized.

Show comment
Hide comment
@Perezmarc

Perezmarc Aug 1, 2016

I thought a gzipped code could be deployed somehow and reduce load time, but probably it makes no sense... Probably my only option is to use angular-universal and render on server to reduce that 1.6 seconds? and lazy loading.

I found this article: http://blog.angular-university.io/angular-2-universal-meet-the-internet-of-the-future-seo-friendly-single-page-web-apps/
I was about to implement the prerendering on S3. That should reduce load time, as I understand

Perezmarc commented Aug 1, 2016

I thought a gzipped code could be deployed somehow and reduce load time, but probably it makes no sense... Probably my only option is to use angular-universal and render on server to reduce that 1.6 seconds? and lazy loading.

I found this article: http://blog.angular-university.io/angular-2-universal-meet-the-internet-of-the-future-seo-friendly-single-page-web-apps/
I was about to implement the prerendering on S3. That should reduce load time, as I understand

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 14, 2016

Owner

FYI I just added experimental support of AoT compilation support bf5a332. It'll be great to test it and once we feel that it is stable enough, merge with the master branch.

Owner

mgechev commented Aug 14, 2016

FYI I just added experimental support of AoT compilation support bf5a332. It'll be great to test it and once we feel that it is stable enough, merge with the master branch.

@mgechev mgechev self-assigned this Aug 14, 2016

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 14, 2016

Owner

Also keep in mind that it may not work on Windows angular/angular#8582.

Owner

mgechev commented Aug 14, 2016

Also keep in mind that it may not work on Windows angular/angular#8582.

@SekibOmazic

This comment has been minimized.

Show comment
Hide comment
@SekibOmazic

SekibOmazic Aug 14, 2016

@mgechev It works fine on Mac, no problems so far. Bundle size is now 1.1M (minified but not gzipped) which is still quite big. Will this be further optimised?

Another proposal: create 3 bundles in prod build: shims, vendor (angular) and app.

SekibOmazic commented Aug 14, 2016

@mgechev It works fine on Mac, no problems so far. Bundle size is now 1.1M (minified but not gzipped) which is still quite big. Will this be further optimised?

Another proposal: create 3 bundles in prod build: shims, vendor (angular) and app.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 14, 2016

Owner

The ahead of time compilation doesn't reduce the bundle size. Once the seed includes Google Closure Compiler we can drop the size more than 10 times, thanks fo tree-shaking and compression (compression we can use today as well).

Owner

mgechev commented Aug 14, 2016

The ahead of time compilation doesn't reduce the bundle size. Once the seed includes Google Closure Compiler we can drop the size more than 10 times, thanks fo tree-shaking and compression (compression we can use today as well).

@sfabriece

This comment has been minimized.

Show comment
Hide comment
@sfabriece

sfabriece Aug 14, 2016

Contributor

Shouldn't it reduce size due to the compiler not being included?

Contributor

sfabriece commented Aug 14, 2016

Shouldn't it reduce size due to the compiler not being included?

@vyakymenko

This comment has been minimized.

Show comment
Hide comment
@vyakymenko

vyakymenko Aug 14, 2016

Contributor

@sfabriece , don't forget about gzip.

Contributor

vyakymenko commented Aug 14, 2016

@sfabriece , don't forget about gzip.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 14, 2016

Owner

@sfabriece correct, which is still significant improvement. By this we get 0.5M off - 1.6M -> 1.1M.

Today I'll work on integration of Google Closure Compiler which should reduce the size even more, although we won't be able to perform tree-shaking yet.

Owner

mgechev commented Aug 14, 2016

@sfabriece correct, which is still significant improvement. By this we get 0.5M off - 1.6M -> 1.1M.

Today I'll work on integration of Google Closure Compiler which should reduce the size even more, although we won't be able to perform tree-shaking yet.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 20, 2016

Owner

FYI we now have Google Closure Compiler support e8105f1. I'd recommend you to give it a try and let me know if you face any issues. The entire JavaScript payload now is ~170K and will drop dramatically once GCC supports export *.

Owner

mgechev commented Aug 20, 2016

FYI we now have Google Closure Compiler support e8105f1. I'd recommend you to give it a try and let me know if you face any issues. The entire JavaScript payload now is ~170K and will drop dramatically once GCC supports export *.

@mpetkov

This comment has been minimized.

Show comment
Hide comment
@mpetkov

mpetkov Aug 25, 2016

Contributor

Hey all, I just read through this entire thread and other threads about lazy-loading/async routing. Wanted to find out if there is any timeframe on when it will be implemented?

I am currently working on application that has a single codebase but many seprate screens (routes). Currently the one prod bundle is getting too big so I am forced to create separate projects for each route to improve loading performance.

Contributor

mpetkov commented Aug 25, 2016

Hey all, I just read through this entire thread and other threads about lazy-loading/async routing. Wanted to find out if there is any timeframe on when it will be implemented?

I am currently working on application that has a single codebase but many seprate screens (routes). Currently the one prod bundle is getting too big so I am forced to create separate projects for each route to improve loading performance.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Aug 25, 2016

Owner

@mpetkov the first step is to reduce the size of the production build. Currently it's 150k (gzipped), it should drop to ~50-70K once we get the tree-shaking work properly.

Owner

mgechev commented Aug 25, 2016

@mpetkov the first step is to reduce the size of the production build. Currently it's 150k (gzipped), it should drop to ~50-70K once we get the tree-shaking work properly.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 13, 2016

Owner

Yes, I think running closure build with preprocessed with tsickle code is possible. I have played with tsickle recently and given that it's wrapper around tsc the integration with the seed should be relatively straightforward.

Here's a post about my experience http://blog.mgechev.com/2016/07/21/even-smaller-angular2-applications-closure-tree-shaking/.

In the "lazy" branch I am also playing with lazy loading which given the bundle arithmetic of SystemJS builder should not be that hard to implement neither. I am not sure how much time I will be able to spend on this in the next a couple of days/by the end of the month.

If anyone is interested in implementing any of these tasks we can chat and I can share some pointers.

Owner

mgechev commented Oct 13, 2016

Yes, I think running closure build with preprocessed with tsickle code is possible. I have played with tsickle recently and given that it's wrapper around tsc the integration with the seed should be relatively straightforward.

Here's a post about my experience http://blog.mgechev.com/2016/07/21/even-smaller-angular2-applications-closure-tree-shaking/.

In the "lazy" branch I am also playing with lazy loading which given the bundle arithmetic of SystemJS builder should not be that hard to implement neither. I am not sure how much time I will be able to spend on this in the next a couple of days/by the end of the month.

If anyone is interested in implementing any of these tasks we can chat and I can share some pointers.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 13, 2016

Owner

Btw, from both tasks, lazy loading seems higher priority, easier to implement and won't vary much in near future.

Owner

mgechev commented Oct 13, 2016

Btw, from both tasks, lazy loading seems higher priority, easier to implement and won't vary much in near future.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 13, 2016

Contributor

I'm looking into the lazy loading part with what you have done so far. I always get this error message

Cannot find 'default' in 'app/+foo/foo.module.js'

I also tried it without export default and attaching #FooModule at the end of the path, but still no dice. Anyone who knows more about this issue?

Contributor

SamVerschueren commented Oct 13, 2016

I'm looking into the lazy loading part with what you have done so far. I always get this error message

Cannot find 'default' in 'app/+foo/foo.module.js'

I also tried it without export default and attaching #FooModule at the end of the path, but still no dice. Anyone who knows more about this issue?

@RoxKilly

This comment has been minimized.

Show comment
Hide comment
@RoxKilly

RoxKilly Oct 13, 2016

Contributor

@SamVerschueren do you have an import statement without brackets anywhere? eg:

import MyModule from 'app/my.module';

Should be

import {MyModule} from 'app/my.module';
Contributor

RoxKilly commented Oct 13, 2016

@SamVerschueren do you have an import statement without brackets anywhere? eg:

import MyModule from 'app/my.module';

Should be

import {MyModule} from 'app/my.module';
@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 14, 2016

Contributor

@RoxKilly With lazy loading, you don't have things like import * from 'app/my.module' because that will actually already import the code and that would mean you can't break those pieces into 2.

This means you have something like this

@NgModule({})
export default FooModule { }
const routes: Routes = [
    { path: 'foo', loadChildren: 'app/+foo/foo.module' }
];

This results in the error

Cannot find 'default' in 'app/+foo/foo.module.js'

Someone at the Angular2 gitter said that export default does not work with AoT. Not sure about that though, but in order to make sure it works I tried it like this.

@NgModule({})
export FooModule { }
const routes: Routes = [
    { path: 'foo', loadChildren: 'app/+foo/foo.module#FooModule' }
];

Which results in

Cannot find 'FooModule' in 'app/+foo/foo.module.js'

I already read through this thread and tried most of the suggestions, no dice.

Contributor

SamVerschueren commented Oct 14, 2016

@RoxKilly With lazy loading, you don't have things like import * from 'app/my.module' because that will actually already import the code and that would mean you can't break those pieces into 2.

This means you have something like this

@NgModule({})
export default FooModule { }
const routes: Routes = [
    { path: 'foo', loadChildren: 'app/+foo/foo.module' }
];

This results in the error

Cannot find 'default' in 'app/+foo/foo.module.js'

Someone at the Angular2 gitter said that export default does not work with AoT. Not sure about that though, but in order to make sure it works I tried it like this.

@NgModule({})
export FooModule { }
const routes: Routes = [
    { path: 'foo', loadChildren: 'app/+foo/foo.module#FooModule' }
];

Which results in

Cannot find 'FooModule' in 'app/+foo/foo.module.js'

I already read through this thread and tried most of the suggestions, no dice.

@RoxKilly

This comment has been minimized.

Show comment
Hide comment
@RoxKilly

RoxKilly Oct 14, 2016

Contributor

@SamVerschueren

export FooModule { }

Should be

export class FooModule {}
Contributor

RoxKilly commented Oct 14, 2016

@SamVerschueren

export FooModule { }

Should be

export class FooModule {}
@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 14, 2016

Contributor

Jup sorry, was just a typo here. I have all that and the lazy loading works in development so that doesn't seem to be the problem. The problem is when building the bundles.

Apparently SystemJS has a bundles option in the config. But, by default, angular-seed does not provide a config for production build as it is a static build. So I started adding the config directly in index.html with the bundles property and I get this far that it actually succesfully loads the module and shows this error

Uncaught (in promise): Error: No NgModule metadata found for 'FooModule'.

I might be getting closer to a solution but do I really have to generate that metadata.json file per bundle? Not sure how to do that either so if someone else could point me in the right direction...

Contributor

SamVerschueren commented Oct 14, 2016

Jup sorry, was just a typo here. I have all that and the lazy loading works in development so that doesn't seem to be the problem. The problem is when building the bundles.

Apparently SystemJS has a bundles option in the config. But, by default, angular-seed does not provide a config for production build as it is a static build. So I started adding the config directly in index.html with the bundles property and I get this far that it actually succesfully loads the module and shows this error

Uncaught (in promise): Error: No NgModule metadata found for 'FooModule'.

I might be getting closer to a solution but do I really have to generate that metadata.json file per bundle? Not sure how to do that either so if someone else could point me in the right direction...

@RoxKilly

This comment has been minimized.

Show comment
Hide comment
@RoxKilly

RoxKilly Oct 14, 2016

Contributor

@SamVerschueren Oh I see. it's only when you bundle that it stops working. Then you've made it further than I. I gave up on lazy loading (for now) because I couldn't get it to work with my bundling build.

Contributor

RoxKilly commented Oct 14, 2016

@SamVerschueren Oh I see. it's only when you bundle that it stops working. Then you've made it further than I. I gave up on lazy loading (for now) because I couldn't get it to work with my bundling build.

@doapp-ryanp

This comment has been minimized.

Show comment
Hide comment
@doapp-ryanp

doapp-ryanp Oct 14, 2016

Contributor

Is it possible to gzip all .html,.css,.js as part of build.prod so they can be directly uploaded to static object store (s3+CloudFront)?

We don't support web clients that don't support Accept-Encoding: gzip

I should note that you may be apt to say let CloudFront gzip the assets, however if CF is busy it wont compress.

Contributor

doapp-ryanp commented Oct 14, 2016

Is it possible to gzip all .html,.css,.js as part of build.prod so they can be directly uploaded to static object store (s3+CloudFront)?

We don't support web clients that don't support Accept-Encoding: gzip

I should note that you may be apt to say let CloudFront gzip the assets, however if CF is busy it wont compress.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 14, 2016

Contributor

@doapp-ryanp Seems like an "easy" extra step that can be written as custom task, no? Seems like a quite specific use case.

Contributor

SamVerschueren commented Oct 14, 2016

@doapp-ryanp Seems like an "easy" extra step that can be written as custom task, no? Seems like a quite specific use case.

@doapp-ryanp

This comment has been minimized.

Show comment
Hide comment
@doapp-ryanp

doapp-ryanp Oct 14, 2016

Contributor

Yup just didn't know if was already build in (and I was just missing a flag/opt). Seems like a very common usage pattern. I will make a PR at some point to add.

Contributor

doapp-ryanp commented Oct 14, 2016

Yup just didn't know if was already build in (and I was just missing a flag/opt). Seems like a very common usage pattern. I will make a PR at some point to add.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 14, 2016

Owner

Hey @doapp-ryanp, I think @SamVerschueren has a point. Lets keep this out of the seed, and maybe write a blog post/wiki article. I recently got rid of 20% of the dependencies and I'm willing to keep this process of shrinking the seed so lets not introduce new dependencies for now.

Owner

mgechev commented Oct 14, 2016

Hey @doapp-ryanp, I think @SamVerschueren has a point. Lets keep this out of the seed, and maybe write a blog post/wiki article. I recently got rid of 20% of the dependencies and I'm willing to keep this process of shrinking the seed so lets not introduce new dependencies for now.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 14, 2016

Contributor

@mgechev If you have any docs that I could use in order to make my SystemJS build work with lazy loading, feel free to provide that information. I'm really struggling for hours, if not days, with this problem. Would love to have this in the seed by default.

Contributor

SamVerschueren commented Oct 14, 2016

@mgechev If you have any docs that I could use in order to make my SystemJS build work with lazy loading, feel free to provide that information. I'm really struggling for hours, if not days, with this problem. Would love to have this in the seed by default.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 14, 2016

Owner

@SamVerschueren here's a demo in the official Angular repo. If it still doesn't work after that we can dig deeper into it.

Owner

mgechev commented Oct 14, 2016

@SamVerschueren here's a demo in the official Angular repo. If it still doesn't work after that we can dig deeper into it.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 14, 2016

Contributor

The thing is that it works while developing (npm start). But the moment I try to create self executing bundles, something fails and I just can't get it to work.

The question is, can we still use builder.buildStatic here or should we use builder.bundle together with a System.import in index.html instead.

Contributor

SamVerschueren commented Oct 14, 2016

The thing is that it works while developing (npm start). But the moment I try to create self executing bundles, something fails and I just can't get it to work.

The question is, can we still use builder.buildStatic here or should we use builder.bundle together with a System.import in index.html instead.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 14, 2016

Owner

From what I left in the lazy branch I remember that I had to exclude Angular from the production lazy loaded bundles. I think this might have led to some issues in non-recognizing metadata. I've had this issue in the past when I run Angular Universal in Service Workers.

Owner

mgechev commented Oct 14, 2016

From what I left in the lazy branch I remember that I had to exclude Angular from the production lazy loaded bundles. I think this might have led to some issues in non-recognizing metadata. I've had this issue in the past when I run Angular Universal in Service Workers.

@scaryroy

This comment has been minimized.

Show comment
Hide comment
@scaryroy

scaryroy Oct 17, 2016

@mgechev so are you saying your lazy branch will work if we dont use export *?

scaryroy commented Oct 17, 2016

@mgechev so are you saying your lazy branch will work if we dont use export *?

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 18, 2016

Owner

No, the lazy branch doesn't use Google Closure Compiler. The work there is WIP, I'll keep working on it once I come back from connect.js.

Owner

mgechev commented Oct 18, 2016

No, the lazy branch doesn't use Google Closure Compiler. The work there is WIP, I'll keep working on it once I come back from connect.js.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Oct 27, 2016

Owner

@SamVerschueren here's a working version. Next step is to make it work with AoT.

Owner

mgechev commented Oct 27, 2016

@SamVerschueren here's a working version. Next step is to make it work with AoT.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Oct 27, 2016

Contributor

@mgechev You are a hero! Thanks for looking into this and finding what caused the issues. I didn't tried it yet but I will when I find some more time.

Contributor

SamVerschueren commented Oct 27, 2016

@mgechev You are a hero! Thanks for looking into this and finding what caused the issues. I didn't tried it yet but I will when I find some more time.

@ruffiem

This comment has been minimized.

Show comment
Hide comment
@ruffiem

ruffiem Nov 4, 2016

Contributor

@mgechev I can't make external libraries work in prod on the lazy branch, is it normal (at this stage) ?

Contributor

ruffiem commented Nov 4, 2016

@mgechev I can't make external libraries work in prod on the lazy branch, is it normal (at this stage) ?

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Nov 13, 2016

Owner

The lazy branch now supports lazy loading with both AoT and JiT compilation. The code there is just proof-of-concept at this point but most likely I'll work on some advanced build in the next a couple of days.

@ruffiem you need to include the library in the closest common parent of the modules using it. It may not be trivial. In my next commits I'll try to introduce a set of conventions aligned with the style guide, for a directory structure which allows easier bundling with systemjs. It should be fun.

Owner

mgechev commented Nov 13, 2016

The lazy branch now supports lazy loading with both AoT and JiT compilation. The code there is just proof-of-concept at this point but most likely I'll work on some advanced build in the next a couple of days.

@ruffiem you need to include the library in the closest common parent of the modules using it. It may not be trivial. In my next commits I'll try to introduce a set of conventions aligned with the style guide, for a directory structure which allows easier bundling with systemjs. It should be fun.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Nov 26, 2016

Owner

I believe we can close this issue. Improving the production build is an incremental process, dependent on the existing tools. By default SystemJS builder uses rollup anyway, and the lazy loading is something which is already working. In case someone is interested we can discuss it further details in gitter or this issue.

Owner

mgechev commented Nov 26, 2016

I believe we can close this issue. Improving the production build is an incremental process, dependent on the existing tools. By default SystemJS builder uses rollup anyway, and the lazy loading is something which is already working. In case someone is interested we can discuss it further details in gitter or this issue.

@mgechev mgechev closed this Nov 26, 2016

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Nov 26, 2016

Contributor

I thought about creating a new issue for this but it makes more sense to put things here.

I wanted to add treeshaking to my project which started from this seed. So I gave it a go and had something working fairly quickly. BUT, I was importing some CommonJS projects from npm in the following way.

import * as x from 'x';

x();

This is not allowed for Rollup as it will fail with something like Could not call namespace x. So I started investigating this. I got it working by changing that import statement to import x from 'x'; and set the allowSyntheticDefaultImports to true in the tsconfig.json. Production build with AoT and rollup was working perfectly!

But when I ran npm start again, it failed with something like could not find x_1.default as I was now importing the default method. I started to investigate this and came across the inter-format dependencies in the SystemJS docs https://github.com/systemjs/systemjs/blob/master/docs/module-formats.md#inter-format-dependencies. So why wasn't that working for me? I thought SystemJS would hijack the require function and exposes my CJS module as default.

I opened an issue systemjs/systemjs#1502 and got a response. Apparently, for the inter format dependencies to work, you have to transpile your code to system instead of commonjs. But with this came a 3rd issue, module.id does not work anymore as it is CommonJS specific (see issue). I had to change all my module.id calls to __moduleName.

Those steps where required to get Rollup working together with a working dev build.

  • Change all the import * as statements to import statements
  • Transpile to system instead of commonjs
  • Change module.id to __moduleName

Quite some changes though, but this got everything working.

@mgechev Yes SystemJS uses rollup, but not sure how good that works. My build with SystemJS was 1.8 MB and when I used rollup directly, I had 900 kB.

Contributor

SamVerschueren commented Nov 26, 2016

I thought about creating a new issue for this but it makes more sense to put things here.

I wanted to add treeshaking to my project which started from this seed. So I gave it a go and had something working fairly quickly. BUT, I was importing some CommonJS projects from npm in the following way.

import * as x from 'x';

x();

This is not allowed for Rollup as it will fail with something like Could not call namespace x. So I started investigating this. I got it working by changing that import statement to import x from 'x'; and set the allowSyntheticDefaultImports to true in the tsconfig.json. Production build with AoT and rollup was working perfectly!

But when I ran npm start again, it failed with something like could not find x_1.default as I was now importing the default method. I started to investigate this and came across the inter-format dependencies in the SystemJS docs https://github.com/systemjs/systemjs/blob/master/docs/module-formats.md#inter-format-dependencies. So why wasn't that working for me? I thought SystemJS would hijack the require function and exposes my CJS module as default.

I opened an issue systemjs/systemjs#1502 and got a response. Apparently, for the inter format dependencies to work, you have to transpile your code to system instead of commonjs. But with this came a 3rd issue, module.id does not work anymore as it is CommonJS specific (see issue). I had to change all my module.id calls to __moduleName.

Those steps where required to get Rollup working together with a working dev build.

  • Change all the import * as statements to import statements
  • Transpile to system instead of commonjs
  • Change module.id to __moduleName

Quite some changes though, but this got everything working.

@mgechev Yes SystemJS uses rollup, but not sure how good that works. My build with SystemJS was 1.8 MB and when I used rollup directly, I had 900 kB.

@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Nov 26, 2016

Owner

Thanks for the comment! It's great to have this information logged somewhere.

I've considered switching to plain Rollup from SystemJS, however, I'm not sure it's a good idea to move drop SystemJS yet. A lot of people are using plenty of third party CommonJS dependencies and the configuration of rollup can become very complicated.

I've been thinking of having a mixed approach where we support bundling with both SystemJS and rollup. Rollup can be a separate task and we can switch to this build by having something like:

npm run build.prod.exp -- --rollup
npm run build.prod -- --rollup

On the other hand, having tsickle with Google Closure Compiler will produce even smaller bundle and will do everything (bundling, tree-shaking, minification) in a single step...

Owner

mgechev commented Nov 26, 2016

Thanks for the comment! It's great to have this information logged somewhere.

I've considered switching to plain Rollup from SystemJS, however, I'm not sure it's a good idea to move drop SystemJS yet. A lot of people are using plenty of third party CommonJS dependencies and the configuration of rollup can become very complicated.

I've been thinking of having a mixed approach where we support bundling with both SystemJS and rollup. Rollup can be a separate task and we can switch to this build by having something like:

npm run build.prod.exp -- --rollup
npm run build.prod -- --rollup

On the other hand, having tsickle with Google Closure Compiler will produce even smaller bundle and will do everything (bundling, tree-shaking, minification) in a single step...

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Nov 26, 2016

Contributor

Yes switching entirely to Rollup by default wouldn't be a good thing. Especially because this means you would have to use __moduleName instead of module.id and compile to system instead of commonjs. Because all of this, it should be opt-in (if we are going for this).

I added the task below that performs the tree shaking for me in case you're interested.

import * as path from 'path';
import * as gulp from 'gulp';
import * as gulpLoadPlugins from 'gulp-load-plugins';
import * as rollup from 'rollup';
import * as nodeResolve from 'rollup-plugin-node-resolve';
import * as commonjs from 'rollup-plugin-commonjs';
import * as uglify from 'rollup-plugin-uglify';
import * as toPromise from 'stream-to-promise';
import Config from '../../config';
import { templateLocals } from '../../utils';

const plugins = <any>gulpLoadPlugins();

const compileTemplate = () => {
	const stream = gulp.src(path.join(Config.TMP_DIR, '**/*.js'))
		.pipe(plugins.template(templateLocals()))
		.pipe(gulp.dest(Config.TMP_DIR));

	return toPromise(stream);
};

const treeshake = () => {
	return rollup
		.rollup({
			entry: path.join(Config.TMP_DIR, Config.BOOTSTRAP_FACTORY_PROD_MODULE),
			plugins: [
				nodeResolve({
					jsnext: true,
					module: true
				}),
				commonjs({
					include: 'node_modules/**'
				}),
				uglify()
			]
		})
		.then((bundle: any) => {
			return bundle.write({
				format: 'iife',
				dest: path.join(Config.JS_DEST, Config.JS_PROD_APP_BUNDLE)
			});
		});
};

export = (done: any) => {
	compileTemplate()
		.then(() => treeshake())
		.then(() => done())
		.catch((err: any) => done(err));
};
Contributor

SamVerschueren commented Nov 26, 2016

Yes switching entirely to Rollup by default wouldn't be a good thing. Especially because this means you would have to use __moduleName instead of module.id and compile to system instead of commonjs. Because all of this, it should be opt-in (if we are going for this).

I added the task below that performs the tree shaking for me in case you're interested.

import * as path from 'path';
import * as gulp from 'gulp';
import * as gulpLoadPlugins from 'gulp-load-plugins';
import * as rollup from 'rollup';
import * as nodeResolve from 'rollup-plugin-node-resolve';
import * as commonjs from 'rollup-plugin-commonjs';
import * as uglify from 'rollup-plugin-uglify';
import * as toPromise from 'stream-to-promise';
import Config from '../../config';
import { templateLocals } from '../../utils';

const plugins = <any>gulpLoadPlugins();

const compileTemplate = () => {
	const stream = gulp.src(path.join(Config.TMP_DIR, '**/*.js'))
		.pipe(plugins.template(templateLocals()))
		.pipe(gulp.dest(Config.TMP_DIR));

	return toPromise(stream);
};

const treeshake = () => {
	return rollup
		.rollup({
			entry: path.join(Config.TMP_DIR, Config.BOOTSTRAP_FACTORY_PROD_MODULE),
			plugins: [
				nodeResolve({
					jsnext: true,
					module: true
				}),
				commonjs({
					include: 'node_modules/**'
				}),
				uglify()
			]
		})
		.then((bundle: any) => {
			return bundle.write({
				format: 'iife',
				dest: path.join(Config.JS_DEST, Config.JS_PROD_APP_BUNDLE)
			});
		});
};

export = (done: any) => {
	compileTemplate()
		.then(() => treeshake())
		.then(() => done())
		.catch((err: any) => done(err));
};
@mgechev

This comment has been minimized.

Show comment
Hide comment
@mgechev

mgechev Nov 26, 2016

Owner

Looks good! Let's think how it'll be best to add this as part of the seed and introduce bundling with Rollup in the next a couple of weeks :-).

Owner

mgechev commented Nov 26, 2016

Looks good! Let's think how it'll be best to add this as part of the seed and introduce bundling with Rollup in the next a couple of weeks :-).

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Nov 26, 2016

Contributor

Let me know if I can help you out with creating a PR if you know how you want this integrated!

Contributor

SamVerschueren commented Nov 26, 2016

Let me know if I can help you out with creating a PR if you know how you want this integrated!

@Perezmarc

This comment has been minimized.

Show comment
Hide comment
@Perezmarc

Perezmarc Dec 13, 2016

How is the implementation of tree shaking and lazy-loading in production mode going? 😃

Perezmarc commented Dec 13, 2016

How is the implementation of tree shaking and lazy-loading in production mode going? 😃

@ogousa

This comment has been minimized.

Show comment
Hide comment
@ogousa

ogousa Feb 10, 2017

@mgechev I have problems with "lazy" branch (Chrome, Windows 10) while creating production build.
First, I have to use

import { HomeComponent } from '../home/home.component';
import { HomeRoutingModule } from '../home/home-routing.module';

instead of

import { HomeComponent } from './home.component';
import { HomeRoutingModule } from './home-routing.module';

The same trick for about module. Otherwise build.lazy-bundles.app.ts cannot find home folder:
image

With these changes I can create bundles, but when I copy result to my web server I have this error:

image

Can you please help me?
Thanks.

ogousa commented Feb 10, 2017

@mgechev I have problems with "lazy" branch (Chrome, Windows 10) while creating production build.
First, I have to use

import { HomeComponent } from '../home/home.component';
import { HomeRoutingModule } from '../home/home-routing.module';

instead of

import { HomeComponent } from './home.component';
import { HomeRoutingModule } from './home-routing.module';

The same trick for about module. Otherwise build.lazy-bundles.app.ts cannot find home folder:
image

With these changes I can create bundles, but when I copy result to my web server I have this error:

image

Can you please help me?
Thanks.

@klesky

This comment has been minimized.

Show comment
Hide comment
@klesky

klesky Apr 3, 2017

hi am having the same error as @ogousa is having, anyone can shed more light into this?

klesky commented Apr 3, 2017

hi am having the same error as @ogousa is having, anyone can shed more light into this?

@negberts

This comment has been minimized.

Show comment
Hide comment
@negberts

negberts May 15, 2017

Same here. Trying to get the Lazy Loading thing working but encountering the same errors (also when using the lazy branch). @mgechev did an update of systemjs or angular maybe caused this?

The error @ogousa got is coming from this line in the bundle.lazy task:
appendFileSync(outpath,\nSystem.import('${mainpath}.js');${addExtensions});
mainpath is pointing to tmp dir. why is this included?

negberts commented May 15, 2017

Same here. Trying to get the Lazy Loading thing working but encountering the same errors (also when using the lazy branch). @mgechev did an update of systemjs or angular maybe caused this?

The error @ogousa got is coming from this line in the bundle.lazy task:
appendFileSync(outpath,\nSystem.import('${mainpath}.js');${addExtensions});
mainpath is pointing to tmp dir. why is this included?

@negberts

This comment has been minimized.

Show comment
Hide comment
@negberts

negberts May 15, 2017

@mgechev what settings for MINI_APP_MODULE and MINI_APP_BUNDLE are you using in this gist: https://gist.github.com/mgechev/63d374d51ac846797879a0cdc9c3b97c ?

negberts commented May 15, 2017

@mgechev what settings for MINI_APP_MODULE and MINI_APP_BUNDLE are you using in this gist: https://gist.github.com/mgechev/63d374d51ac846797879a0cdc9c3b97c ?

@negberts

This comment has been minimized.

Show comment
Hide comment
@negberts

negberts May 28, 2017

@ogousa and @klesky did you get any further? Your problem is that in app.js the module is generated as "dist\\tmp\\app\\main.js" but is imported as "dist\tmp\app\main.js". Thats a problem voor SystemJs.
After I fixed that I received another error complaining about app.module and when I have a look in chrome to the source of app.module it shows me the content of index.html:

<!DOCTYPE html><html lang="en"><head><base href="/"><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=edge"><title>PIM Portal</title><meta name="description" content=""><meta name="viewport" content="width=device-width,initial-scale=1"><link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet"><link rel="stylesheet" href="/css/main.css?1496000579136"></head><body><pim-app>Loading...</pim-app><script>function module(){}</script><script src="/js/shims.js?1496000579131"></script><script src="/js/app.js?1496000579133"></script></body></html>

Really strange...

negberts commented May 28, 2017

@ogousa and @klesky did you get any further? Your problem is that in app.js the module is generated as "dist\\tmp\\app\\main.js" but is imported as "dist\tmp\app\main.js". Thats a problem voor SystemJs.
After I fixed that I received another error complaining about app.module and when I have a look in chrome to the source of app.module it shows me the content of index.html:

<!DOCTYPE html><html lang="en"><head><base href="/"><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=edge"><title>PIM Portal</title><meta name="description" content=""><meta name="viewport" content="width=device-width,initial-scale=1"><link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet"><link rel="stylesheet" href="/css/main.css?1496000579136"></head><body><pim-app>Loading...</pim-app><script>function module(){}</script><script src="/js/shims.js?1496000579131"></script><script src="/js/app.js?1496000579133"></script></body></html>

Really strange...

@klesky

This comment has been minimized.

Show comment
Hide comment
@klesky

klesky Jul 4, 2017

@negberts i still could not find a solution for this
and what do you mean by "module is generated as "dist\tmp\app\main.js" but is imported as "dist\tmp\app\main.js"." ?

klesky commented Jul 4, 2017

@negberts i still could not find a solution for this
and what do you mean by "module is generated as "dist\tmp\app\main.js" but is imported as "dist\tmp\app\main.js"." ?

@negberts

This comment has been minimized.

Show comment
Hide comment
@negberts

negberts Jul 4, 2017

"dist\\tmp\\app\\main.js" but is imported as "dist\tmp\app\main.js".

The slashes were removed in my comment. System.js sees \tmp as an escaped t, but the \ should be escaped by \\

negberts commented Jul 4, 2017

"dist\\tmp\\app\\main.js" but is imported as "dist\tmp\app\main.js".

The slashes were removed in my comment. System.js sees \tmp as an escaped t, but the \ should be escaped by \\

@darecstowell

This comment has been minimized.

Show comment
Hide comment
@darecstowell

darecstowell Jul 14, 2017

Having the same issues as @ogousa and @klesky on the lazy branch. That branch could use a bit of love. Are there any plans to update it to Angular 4? My org is creating a fairly large app that benefits more from lazy loading than it does Tree Shaking/Rollup on master.

darecstowell commented Jul 14, 2017

Having the same issues as @ogousa and @klesky on the lazy branch. That branch could use a bit of love. Are there any plans to update it to Angular 4? My org is creating a fairly large app that benefits more from lazy loading than it does Tree Shaking/Rollup on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment