fix(@angular-devkit/build-angular): fix budget checks #16268
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alan-agius4
reviewed
Nov 23, 2019
packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts
Outdated
Show resolved
Hide resolved
clydin
requested changes
Nov 24, 2019
packages/angular_devkit/build_angular/src/angular-cli-files/models/webpack-configs/common.ts
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts
Show resolved
Hide resolved
clydin
reviewed
Nov 25, 2019
packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts
Outdated
Show resolved
Hide resolved
dgp1130
force-pushed
the
budgets
branch
4 times, most recently
from
e9a2408
to
4a9543d
Compare
clydin
reviewed
Nov 27, 2019
clydin
reviewed
Nov 27, 2019
dgp1130
force-pushed
the
budgets
branch
3 times, most recently
from
ec36dc8
to
3b91986
Compare
dgp1130
force-pushed
the
budgets
branch
3 times, most recently
from
4e4730e
to
c43a5a9
Compare
clydin
approved these changes
Dec 9, 2019
…be post-build Refs angular#15792. This provides access to all the size information necessary because all build steps have already completed. This commit is roughly a no-op because it simply moves the budget checks (for different builds) to be executed post-build. The lone exception is the AnyComponentStyle budget. Component stylesheet files are not emitted after the build is completed, so there is no size information to work with. Instead, these budgets are checked during a separate plugin (exected for different builds **and** non-differential builds).
… bundle budget. Refs angular#15792. Static files listed in `angular.json` were being accounted in the `initial` bundle budget even when they were deferred asynchronously with `"lazy": true` or `"inject": false`. Webpack belives these files to be `initial`, so this commit corrects that by finding all extra entry points and excluding ones which are explicitly marked by the application developer as asynchronous. One edge case would be that the main bundle might transitively depend on one of these static files, and thus pull it into the `initial` bundle. However, this is not possible because the files are not present until the end of the build and cannot be depended upon by a Webpack build step. Thus all files listed by the application developer can be safely assumed to truly be loaded asynchronously.
…ial builds separately Fixes angular#15792. Previously, budgets would include content for both versions of a differential build. Thus the `initial` budget would count content from the ES5 **and** ES2015 bundles together. This is a very misleading statistic because no user would download both versions. I've updated the budget calculators to take this into account and generate size values for both builds which are then checked independently of each other. The only calculators I changed are the `InitialCalculator` (for computing `initial` bundle sizes) and `BundleCalculator` (for computing named bundles). Since budgets are handled by Webpack for builds without differential loading, the `initial` bundle will always have those two sizes. The `BundleCalculator` might reference a bundle which does not have differential loading performed (such as a CSS file), so it emits sizes depending on whether or not multiple builds were found for that chunk. Most of the other calculators don't really need to take differential loading into account. `AnyScriptCalculator` and `AnyCalculator` already apply on a file-by-file basis, so they generate sizes for both build versions already. `AnyComponentStyleCalculator` only applies to CSS which does not have differential builds. The wierd ones here are `AllCalculator` and `AllScriptCalculator` which reference files with and without differential builds. Conceptually, they should be separated, as a "total" budget specified by an app developer probably wanted it to mean "the total resources a user would have to download", which would only be one differential build at a time. However, I don't see a good way of identifying which assets belong to which differential build. Even if an asset belongs to a chunk with differential builds, we don't know which build takes which assets into account. I decided to leave this for the time being, but it is probably something we should look into separately. Since budgets take differential loading into account, users might reasonably want to set different budgets for different builds (ie. "initial-es2015 bundle should be capped at 100k, but initial-es5 bundle can go to 150k"). That's more of a feature request, so I also left that out for a future PR.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #15792.
This turned out to be a little more complicated than expected due to the combination of a few different problems resulting in broken budget checks.
lazy: trueorinject: falsein the app dev'sangular.jsonfile were included in theinitialbundle, even though these resources must be loaded asynchronously. The second commit filters out files which were marked asynchronous from theinitialbundle.mainsize was including both the ES2015 and ES5 sizes together, which isn't what the user would want. The third commit updates the relevant budgets to output two sizes, one for each differential build, rather than including them together.I made separate commits performing each of these steps individually to make the logic a little easier to follow and understand.