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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.3.0-rc.0] Sourcemaps seem borken when using new build optimizer #7093

Open
bboehm86 opened this Issue Jul 22, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@bboehm86

bboehm86 commented Jul 22, 2017

First of big thanks to @filipesilva for adding the --build-optimizer! It saved about 200kb (non gzip) in overall size on a blank project with material and about 500kb on a production one, good job 馃憤

(as a note: up until now I could not see any real size difference when enabling the vendor chunk again, always <1kb)

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Versions.

    _                      _                 ____ _     ___
   / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
  / 鈻 \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
 / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
/_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
               |___/
@angular/cli: 1.3.0-rc.0
node: 8.1.1
os: win32 x64
@angular/animations: 4.3.1
@angular/cdk: 2.0.0-beta.8
@angular/common: 4.3.1
@angular/compiler: 4.3.1
@angular/core: 4.3.1
@angular/forms: 4.3.1
@angular/http: 4.3.1
@angular/material: 2.0.0-beta.8
@angular/platform-browser: 4.3.1
@angular/platform-browser-dynamic: 4.3.1
@angular/router: 4.3.1
@angular/cli: 1.3.0-rc.0
@angular/compiler-cli: 4.3.1
@angular/language-service: 4.3.1

on Windows 10

Repro steps.

Install latest @angular/cli@1.3.0-rc.0,
generate a new project and
run ng build --prod --bo --oh=none --vendor-chunk=true. (same goes when vendor chunk is disabled)
change to dist and run source-map-explorer vendor.bundle.js (same for webpack-bundle-analyzer)

or use https://github.com/bboehm86/mat-chunk-test
run npm run build:bo:vc

The log given by the failure.

No failure but just no usable informations for vendor files (there should be angular, angular material and rxjs inside)
cli

Desired functionality.

It would be nice if still could still have usabe sourcemap infos to take a look into our bundle and see whats inside :-)

@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Jul 22, 2017

Member

Thanks a bunch @bboehm86, this is super useful info - especially with easy repro and numbers. This is exactly the stuff I want to get fixed for 1.3.0. I'll get on it.

Regarding size differences, they were there but the stats didn't show it. We fixed the stats in #6989, which went in rc.0 as well.

Member

filipesilva commented Jul 22, 2017

Thanks a bunch @bboehm86, this is super useful info - especially with easy repro and numbers. This is exactly the stuff I want to get fixed for 1.3.0. I'll get on it.

Regarding size differences, they were there but the stats didn't show it. We fixed the stats in #6989, which went in rc.0 as well.

@bboehm86

This comment has been minimized.

Show comment
Hide comment
@bboehm86

bboehm86 Jul 22, 2017

Thanks a lot for the quick response, you guys are rly doing a great job!

As for the size differences.. I guess you are refering to the vendor chunk disbaled/enabled option. I checked the actual size of the files for that (main.bundle.js and vendor.bundle.js) since I was used to do that anyways and didn't look at the console output. (but I was aware of the fix of the stats output) :-)
Since you like numbers I took a look again. So for the example repo it would be:

  • vc disabled: 708.961 bytes [with gzip: 153.871 bytes] (main.bundle.js)
  • vc enabled: 709.010 bytes [with gzip: 153.919 bytes] (main.bundle.js & vendor.bundle.js)

Not that big a difference (for this example at least)

bboehm86 commented Jul 22, 2017

Thanks a lot for the quick response, you guys are rly doing a great job!

As for the size differences.. I guess you are refering to the vendor chunk disbaled/enabled option. I checked the actual size of the files for that (main.bundle.js and vendor.bundle.js) since I was used to do that anyways and didn't look at the console output. (but I was aware of the fix of the stats output) :-)
Since you like numbers I took a look again. So for the example repo it would be:

  • vc disabled: 708.961 bytes [with gzip: 153.871 bytes] (main.bundle.js)
  • vc enabled: 709.010 bytes [with gzip: 153.919 bytes] (main.bundle.js & vendor.bundle.js)

Not that big a difference (for this example at least)

@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Jul 22, 2017

Member

I digged into the sourcemaps and we're definitely breaking them somewhere, and also the default was broken (prod build should have it turned off by default).

It's cool to see that the VC didn't make much of a difference in your setup. In another setup (angular.io actually) it made a huge difference. About 40% of the total savings were gone by having the VS. But I hope that webpack improvements in scope hoisting might bring that down. It's something we'll keep monitoring.

Member

filipesilva commented Jul 22, 2017

I digged into the sourcemaps and we're definitely breaking them somewhere, and also the default was broken (prod build should have it turned off by default).

It's cool to see that the VC didn't make much of a difference in your setup. In another setup (angular.io actually) it made a huge difference. About 40% of the total savings were gone by having the VS. But I hope that webpack improvements in scope hoisting might bring that down. It's something we'll keep monitoring.

@bboehm86

This comment has been minimized.

Show comment
Hide comment
@bboehm86

bboehm86 Jul 22, 2017

@filipesilva Good point you've got there it slipped my mind that they are disabled by default...

And you are quite right about the vendor chunk size at aio. I tried it with the version you commited with the new CLI and I'm getting the same differences in size you've mentioned. Not sure why though...
If you are going to investigate this, it might help you that I found some Material assets in the vendor chunk that shouldn't be there (as they aren't present in the version without the vendor chunk).
eg. the MdDatepicker gets removed while building since it's not used. But when using the vc the error messages, the default month names, etc. are still present in the vendor chunk file (while getting remove when disabling the vc)...

bboehm86 commented Jul 22, 2017

@filipesilva Good point you've got there it slipped my mind that they are disabled by default...

And you are quite right about the vendor chunk size at aio. I tried it with the version you commited with the new CLI and I'm getting the same differences in size you've mentioned. Not sure why though...
If you are going to investigate this, it might help you that I found some Material assets in the vendor chunk that shouldn't be there (as they aren't present in the version without the vendor chunk).
eg. the MdDatepicker gets removed while building since it's not used. But when using the vc the error messages, the default month names, etc. are still present in the vendor chunk file (while getting remove when disabling the vc)...

@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Jul 22, 2017

Member

Your findings corroborate what I've been seeing as well. I think it's wholly related to scope hoisting and the current status of angular material tree shaking (angular/material2#4137).

So by itself material is not tree-shaked properly; but with scope hoisting + uglify + build-optimizer, parts of it are. Scope hoisting is very important here as it helps uglify drop more stuff. When you have a vendor chunk, there should be at least 2 scope hoisting contexts (one for main, or for vendor). Having them separated reduces gains by not allowing uglify to drop as much.

This seems to be more important when the code is stuff to which build optimizer adds annotations (downleved TS code and angular specific code). So when you have a lot of angular libs in the same chunk, more is saved.

If you have a lot of stuff that the build optimizer doesn't touch, the difference isn't as dramatic.

Member

filipesilva commented Jul 22, 2017

Your findings corroborate what I've been seeing as well. I think it's wholly related to scope hoisting and the current status of angular material tree shaking (angular/material2#4137).

So by itself material is not tree-shaked properly; but with scope hoisting + uglify + build-optimizer, parts of it are. Scope hoisting is very important here as it helps uglify drop more stuff. When you have a vendor chunk, there should be at least 2 scope hoisting contexts (one for main, or for vendor). Having them separated reduces gains by not allowing uglify to drop as much.

This seems to be more important when the code is stuff to which build optimizer adds annotations (downleved TS code and angular specific code). So when you have a lot of angular libs in the same chunk, more is saved.

If you have a lot of stuff that the build optimizer doesn't touch, the difference isn't as dramatic.

@bboehm86

This comment has been minimized.

Show comment
Hide comment
@bboehm86

bboehm86 Jul 22, 2017

I see, great explanation thanks al lot 馃憤

bboehm86 commented Jul 22, 2017

I see, great explanation thanks al lot 馃憤

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Aug 3, 2017

@hansl hansl closed this in #7262 Aug 3, 2017

hansl added a commit that referenced this issue Aug 3, 2017

hansl added a commit that referenced this issue Aug 3, 2017

@ianchi

This comment has been minimized.

Show comment
Hide comment
@ianchi

ianchi Aug 8, 2017

Hi @filipesilva, I'm not sure if this is related to this error, but now source map explorer is working again, but I get strange results in the reported sizes.
For instance my "main.bundle.js" file is 683kb (not gziped) but the reporting in the source map explorer is only 297kb.
Is it possible that we are still loosing some info in the source map?
( this was done with rc.5)

ianchi commented Aug 8, 2017

Hi @filipesilva, I'm not sure if this is related to this error, but now source map explorer is working again, but I get strange results in the reported sizes.
For instance my "main.bundle.js" file is 683kb (not gziped) but the reporting in the source map explorer is only 297kb.
Is it possible that we are still loosing some info in the source map?
( this was done with rc.5)

@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Aug 10, 2017

Member

Will look at it, reopening.

Member

filipesilva commented Aug 10, 2017

Will look at it, reopening.

@filipesilva filipesilva reopened this Aug 10, 2017

svallory added a commit to cashfarm/angular-advanced-cli that referenced this issue Aug 10, 2017

[v1.3.0] Squashed commit of the following:
commit 346e1f0
Author: Saulo Vallory <me@saulovallory.com>
Date:   Thu Aug 10 11:44:29 2017 -0300

    Fix package name and urls and add yarn lock

commit 36cfa66
Author: Saulo Vallory <me@saulovallory.com>
Date:   Mon Jul 31 18:28:23 2017 -0300

    Update package name and dependency check

commit c98c27c
Author: Saulo Vallory <me@saulovallory.com>
Date:   Mon Jul 31 16:08:26 2017 -0300

    Update README and package info

    # Conflicts:
    #	package.json

commit e8e0330
Author: Saulo Vallory <me@saulovallory.com>
Date:   Fri Jul 28 01:54:54 2017 -0300

    Remove console.log calls and fix file check

commit 23789e8
Author: Saulo Vallory <me@saulovallory.com>
Date:   Fri Jul 28 00:27:48 2017 -0300

    Allow partially overriding webpack config

    If a webpack.config.js file is found in the project root, it's loaded after all other configs so configuration can be overriden.

commit 5b9eb08
Author: Hans Larsen <hans@hansl.ca>
Date:   Wed Aug 9 16:32:30 2017 -0700

    release: v1.3.0

commit 3271ac0
Author: Hans Larsen <hans@hansl.ca>
Date:   Wed Aug 9 15:43:38 2017 -0700

    fix(@angular/cli): update build-optimizer to latest

    Also fixes linting errors. Seems like tslint was updated.

commit f454eb2
Author: Hans Larsen <hans@hansl.ca>
Date:   Wed Aug 2 14:59:14 2017 -0700

    feat(@ngtools/webpack): fix paths mapping support

    This is a similar version of #5033. Reverted in #6463 because of issue #6451.

    This is a feature because we do not want it in 1.3.0

commit c475cf4
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Sat Aug 5 15:32:42 2017 +0100

    fix(@ngtools/webpack): show TS error message when there is no file

commit 349aae8
Author: kevinphelps <KevinPhelps11@gmail.com>
Date:   Thu Aug 3 12:52:35 2017 -0500

    fix(@angular/cli): use correct property names for build config defaults

commit af2d957
Author: Mike Brocchi <mbrocchi@gmail.com>
Date:   Thu Aug 3 18:24:49 2017 -0400

    fix(@angular/cli): Update @angular-devkit/build-optimizer

commit d69d797
Author: Robin Dijkhof <tardijkhof@gmail.com>
Date:   Wed Jul 12 14:25:30 2017 +0200

    fix(@angular/cli): prevent unicode character compression

commit 80dfb0d
Author: S K (xz64) <xz64.xyz@gmail.com>
Date:   Mon Jul 31 20:21:16 2017 -0700

    fix(@angular/cli): fix empty 3rdpartylicenses.txt

    Upgrades license-webpack-plugin from 0.4.x to 0.5.x.

    Closes #6921

commit 25a228d
Author: Hans Larsen <hans@hansl.ca>
Date:   Thu Aug 3 12:16:03 2017 -0700

    release: 1.3.0-rc.5

commit 94b11f7
Author: Hans Larsen <hans@hansl.ca>
Date:   Thu Aug 3 11:22:22 2017 -0700

    release: 1.3.0-rc.4

commit fe8b7be
Author: Hans Larsen <hans@hansl.ca>
Date:   Thu Aug 3 11:11:26 2017 -0700

    fix(@angular/cli): update package-lock for build-optimizer

commit 38c064c
Author: Hans <hans@hansl.ca>
Date:   Thu Aug 3 10:25:58 2017 -0700

    fix(@angular/cli): update build-optimizer to latest

commit 2486bf9
Author: sendilkumarn <sendilkumarn@live.com>
Date:   Wed Aug 2 19:02:00 2017 +0800

    fix(@angular/cli): baseHref to accept empty string via cli

commit d0cf4a2
Author: Josh Iverson <jiverson222@gmail.com>
Date:   Wed Aug 2 23:52:54 2017 -0700

    docs: add NamedChunks to dev/prod defaults table

commit 323cd97
Author: Charles Lyding <charles@thunderpos.com>
Date:   Thu Aug 3 11:10:02 2017 -0400

    fix(@angular/cli): sync webpack stats file content options

commit 580deaa
Author: Josh Iverson <jiverson222@gmail.com>
Date:   Thu Aug 3 00:07:02 2017 -0700

    docs: fix material install

commit aa0a59b
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Thu Aug 3 18:01:54 2017 +0100

    Fix(@angular/cli): update build-optimizer

    Fix #7114
    Fix #7110
    Fix #7093

commit 555c110
Author: Hans Larsen <hans@hansl.ca>
Date:   Wed Aug 2 13:33:12 2017 -0700

    build: use the tar files for dependencies for e2e

    This adds a new flag to the build script that sets the tar files path as dependencies inside the package.json before tarring the files.

commit 3aecfa7
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Tue Aug 1 19:26:59 2017 +0100

    fix(@angular/cli): remove -bo alias for --build-optimizer

    /cc @StephenFluin

commit 9d56c85
Author: Hans Larsen <hans@hansl.ca>
Date:   Thu Jul 13 11:26:51 2017 -0700

    docs: add more badges

commit 14f9bf6
Author: Justin Grayston <justingrayston@gmail.com>
Date:   Mon Jul 31 15:01:36 2017 +0100

    docs: fix universal dep save flag

    Closes #7211

commit 50b5059
Author: Rakhat Jabagin <neilhem@hotmail.com>
Date:   Sat Jul 22 17:59:37 2017 +0600

    docs: add other possible cases to generate in project readme

commit fd88158
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Thu Jul 20 11:00:19 2017 +0100

    build: use npm5 instead of yarn

commit 64a7ca4
Author: Charles Lyding <charles@thunderpos.com>
Date:   Mon Jul 31 23:47:11 2017 -0400

    refactor(@angular/cli): update TS type dependencies

commit 6cfa7c3
Author: christian.scharr <christian.scharr@gmail.com>
Date:   Mon Jul 31 10:03:14 2017 -0400

    fix(@angular/cli): exclude node_modules while take account for os specific path separators (#6870)

commit 542f2bd
Author: Christian Scharr <christian.scharr@gmail.com>
Date:   Wed Jul 26 20:45:09 2017 +0200

    fix(@angular/cli): use relative path instead of regex to exclude node_modules (#6870)

commit ca98b2b
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Mon Jul 31 15:25:58 2017 +0100

    fix(@angular/cli): show warnings on serve

    Errors and warnings neet to be printed separately.

    Followup to #6989
    Fix #7213

commit 90a14d5
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Fri Jul 21 14:01:35 2017 +0100

    docs: add SUPPORT.md

    Just contains the `Got a Question or Problem?` section of `CONTRIBUTING.md`.

    See https://github.com/blog/2400-support-file-support for github support.

    Didn't add it to `.github` folder because it's more visible outside, like `CONTRIBUTING.md`.

commit df6d331
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Wed Jul 26 15:51:55 2017 +0100

    fix(@angular/cli): use 3 uglify passes with build-optimizer

    PURE comments work best with 3 passes.
    See webpack/webpack#2899 (comment) for context and an example.

    Thanks to @kzc for highlighting this.

commit a8d3806
Author: yokots <hd.wang1993@gmail.com>
Date:   Fri Jul 28 16:29:10 2017 +0800

    docs: apps[0].lintFix => defaults.lintFix

commit 8257149
Author: cexbrayat <cedric@ninja-squad.com>
Date:   Thu Jul 27 07:14:48 2017 +0200

    fix(@angular/cli): change blog link to blog.angular.io

commit 983b470
Author: Aliaksei Kuncevic <kncvch@gmail.com>
Date:   Thu Jul 27 10:02:58 2017 +1000

    fix(@angular/cli): ignore yarn-error.log directory

commit 8e008e8
Author: Mike Brocchi <mbrocchi@gmail.com>
Date:   Fri Jul 21 16:29:21 2017 -0400

    fix(@angular/cli): Include missing dependency when ejecting

commit a6ac43a
Author: Charles Lyding <charles@thunderpos.com>
Date:   Sun Jul 30 23:14:05 2017 -0400

    fix(@angular/cli): standardize output path behavior

commit 29a7e3f
Author: Kristofer Karlsson <karlsson.kristofer@gmail.com>
Date:   Thu Jul 27 23:10:12 2017 +0200

    docs: Fixed typo in universal story

commit 1d9e07e
Author: AntoineC <aecz@users.noreply.github.com>
Date:   Mon Jul 31 12:45:54 2017 +0200

    fix(@angular/cli): add explicit source-map-support dependency (#7191)

    karma plugin relies on source-map-support being a dependency of karma-source-map-support and npm 3+ flat dependency tree.

    pnpm/pnpm#863

commit 27fd95d
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Thu Jul 27 17:14:22 2017 +0100

    release: 1.3.0-rc.3

commit 20c7b2a
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Thu Jul 27 16:53:04 2017 +0100

    Support enhanced resolve (#7169)

    * fix(@ngtools/webpack): support enhanced-resolve@3.4.0

    Followup to #7123

    * fix(@angular/cli): unpin webpack 3.3.0

    Followup to #7130

commit 453f75f
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Wed Jul 26 22:01:40 2017 +0100

    release: 1.3.0-rc.2

commit 0edd99d
Author: Juri Strumpflohner <juri.strumpflohner@gmail.com>
Date:   Fri Jun 23 14:42:28 2017 +0200

    fix(@angular/cli): change blog link to international one

commit 64d4138
Author: Charles Lyding <charles@thunderpos.com>
Date:   Sat Jul 22 13:36:08 2017 -0400

    fix(@angular/cli): prevent .cur file inlining

    IE does not support data URI cursors.

commit 1a6aa53
Author: Charles Lyding <charles@thunderpos.com>
Date:   Sat Jul 22 13:27:53 2017 -0400

    fix(@angular/cli): remove deprecated json-loader

commit dea46c7
Author: Charles Lyding <charles@thunderpos.com>
Date:   Sun Jul 23 10:13:00 2017 -0400

    refactor(@angular/cli): remove magic-string console message

commit 0b86640
Author: Jason Jean <jasonjean1993@gmail.com>
Date:   Wed Jul 26 15:47:49 2017 -0400

    fix(@ngtools/webpack): fix duplicate LAZY_ROUTE_MAP exports (#7107)

commit 53ec7d4
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Tue Jul 25 10:50:44 2017 +0100

    docs: add universal rendering

    Add doc file for https://github.com/angular/angular-cli/wiki/stories-universal-rendering

    /cc @alxhub

commit a674b0c
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Tue Jul 25 10:25:32 2017 +0100

    fix(@angular/cli): pin webpack to 3.3.0

    Webpack is updating to `enhanced-resolve@3.4.0` (webpack/webpack@16bf0b6) but the CLI cannot do so yet (see #7123).

    This PR pins `webpack@3.3.0` until we are able to update to `enhanced-resolve@^3.4.0`.

commit 38f2135
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Mon Jul 24 19:38:31 2017 +0100

    release: 1.3.0-rc.1

commit 97c7cfb
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Mon Jul 24 19:12:38 2017 +0100

    fix(@ngtools/webpack): pin enhanced-resolve

    See angular#7113 and webpack/enhanced-resolve#98 for context.

    This should be unpinned when a real fix is found.

commit 3da774e
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Fri Jul 21 15:45:07 2017 +0100

    ci: use headless chrome on travis

commit 1c04fd5
Author: Filipe Silva <filipematossilva@gmail.com>
Date:   Fri Jul 21 15:31:51 2017 +0100

    ci: use chrome on travis

commit 260db50
Author: Charles Lyding <charles@thunderpos.com>
Date:   Fri Jul 21 21:03:21 2017 -0400

    refactor: general dependency update

commit 7c2cff8
Author: Hans Larsen <hans@hansl.ca>
Date:   Fri Jul 21 15:17:39 2017 -0700

    release: v1.3.0-rc.0
@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Aug 15, 2017

Member

@ianchi with 1.3.0 final I could see a difference, but not directly related to build optimizer. I built the application three times, and compared the disk size of main.bundle.js with the size reported by source-map-explorer:

  • --aot --build-optimizer: 3.57 MB disk-size, 3.26 MB source-map-explorer
  • --prod: 1.1 MB disk-size, 407 kB source-map-explorer
  • --prod --build-optimizer: 540 kB disk-size, 288 kB source-map-explorer

I also tried webpack-bundle-analyzer and it seems to report much closer values for stat/parsed sizes.

So there's always a difference, I'm not sure why. The difference without --prod is rather tame and could possibly be webpack helper functions.

But the difference in prod is around half the size. It seems related to either uglification or webpack concatenated modules (that's what changes in production).

@TheLarkInn do you have any insight on this?

Member

filipesilva commented Aug 15, 2017

@ianchi with 1.3.0 final I could see a difference, but not directly related to build optimizer. I built the application three times, and compared the disk size of main.bundle.js with the size reported by source-map-explorer:

  • --aot --build-optimizer: 3.57 MB disk-size, 3.26 MB source-map-explorer
  • --prod: 1.1 MB disk-size, 407 kB source-map-explorer
  • --prod --build-optimizer: 540 kB disk-size, 288 kB source-map-explorer

I also tried webpack-bundle-analyzer and it seems to report much closer values for stat/parsed sizes.

So there's always a difference, I'm not sure why. The difference without --prod is rather tame and could possibly be webpack helper functions.

But the difference in prod is around half the size. It seems related to either uglification or webpack concatenated modules (that's what changes in production).

@TheLarkInn do you have any insight on this?

dond2clouds added a commit to d2clouds/speedray-cli that referenced this issue Apr 23, 2018

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