Skip to content
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

refactor(@angular-devkit/build-angular): remove BundleBudgetPlugin #19890

Merged
merged 2 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 41 additions & 38 deletions packages/angular_devkit/build_angular/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ export function buildWebpackBrowser(

return { success };
} else {
const processResults: ProcessBundleResult[] = [];
const bundleInfoStats: BundleStats[] = [];
outputPaths = ensureOutputPaths(baseOutputPath, i18n);

Expand Down Expand Up @@ -445,7 +446,6 @@ export function buildWebpackBrowser(

const processActions: typeof actions = [];
let processRuntimeAction: ProcessBundleOptions | undefined;
const processResults: ProcessBundleResult[] = [];
for (const action of actions) {
// If SRI is enabled always process the runtime bundle
// Lazy route integrity values are stored in the runtime bundle
Expand Down Expand Up @@ -598,22 +598,6 @@ export function buildWebpackBrowser(
const asset = webpackStats.assets?.find(a => a.name === chunk.files[0]);
bundleInfoStats.push(generateBundleStats({ ...chunk, size: asset?.size }));
}

// Check for budget errors and display them to the user.
const budgets = options.budgets || [];
const budgetFailures = checkBudgets(budgets, webpackStats, processResults);
for (const { severity, message } of budgetFailures) {
switch (severity) {
case ThresholdSeverity.Warning:
webpackStats.warnings.push(message);
break;
case ThresholdSeverity.Error:
webpackStats.errors.push(message);
break;
default:
assertNever(severity);
}
}
} else {
files = emittedFiles.filter(x => x.name !== 'polyfills-es5');
noModuleFiles = emittedFiles.filter(x => x.name === 'polyfills-es5');
Expand All @@ -636,29 +620,48 @@ export function buildWebpackBrowser(
}
}

// Copy assets
if (!options.watch && options.assets?.length) {
spinner.start('Copying assets...');
try {
await copyAssets(
normalizeAssetPatterns(
options.assets,
root,
normalize(projectRoot),
projectSourceRoot === undefined ? undefined : normalize(projectSourceRoot),
),
Array.from(outputPaths.values()),
context.workspaceRoot,
);
spinner.succeed('Copying assets complete.');
} catch (err) {
spinner.fail(colors.redBright('Copying of assets failed.'));

return { success: false, error: 'Unable to copy assets: ' + err.message };
// Check for budget errors and display them to the user.
const budgets = options.budgets;
if (budgets?.length) {
const budgetFailures = checkBudgets(budgets, webpackStats, processResults);
for (const { severity, message } of budgetFailures) {
switch (severity) {
case ThresholdSeverity.Warning:
webpackStats.warnings.push(message);
break;
case ThresholdSeverity.Error:
webpackStats.errors.push(message);
break;
default:
assertNever(severity);
}
}
}

if (success) {
const buildSuccess = success && !statsHasErrors(webpackStats);
if (buildSuccess) {
// Copy assets
if (!options.watch && options.assets?.length) {
spinner.start('Copying assets...');
try {
await copyAssets(
normalizeAssetPatterns(
options.assets,
root,
normalize(projectRoot),
projectSourceRoot === undefined ? undefined : normalize(projectSourceRoot),
),
Array.from(outputPaths.values()),
context.workspaceRoot,
);
spinner.succeed('Copying assets complete.');
} catch (err) {
spinner.fail(colors.redBright('Copying of assets failed.'));

return { success: false, error: 'Unable to copy assets: ' + err.message };
}
}

if (options.index) {
spinner.start('Generating index html...');

Expand Down Expand Up @@ -735,7 +738,7 @@ export function buildWebpackBrowser(

webpackStatsLogger(context.logger, webpackStats, config, bundleInfoStats);

return { success: !statsHasErrors(webpackStats) };
return { success: buildSuccess };
}
}),
map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import { findAllNodeModules } from '../../utils/find-up';
import { Spinner } from '../../utils/spinner';
import { isWebpackFiveOrHigher, withWebpackFourOrFive } from '../../utils/webpack-version';
import {
BundleBudgetPlugin,
DedupeModuleResolvePlugin,
NamedLazyChunksPlugin,
OptimizeCssWebpackPlugin,
Expand Down Expand Up @@ -361,17 +360,6 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
);
}

if (buildOptions.budgets.length && !differentialLoadingMode) {
// Budgets are computed after differential builds, not via a plugin.
// https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/browser/index.ts
const extraEntryPoints = [
...normalizeExtraEntryPoints(buildOptions.styles || [], 'styles'),
...normalizeExtraEntryPoints(buildOptions.scripts || [], 'scripts'),
];

extraPlugins.push(new BundleBudgetPlugin({ budgets: buildOptions.budgets, extraEntryPoints }));
}

if ((scriptsSourceMap || stylesSourceMap)) {
extraRules.push({
test: /\.m?js$/,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// Exports the webpack plugins we use internally.
export { AnyComponentStyleBudgetChecker } from './any-component-style-budget-checker';
export { OptimizeCssWebpackPlugin, OptimizeCssWebpackPluginOptions } from './optimize-css-webpack-plugin';
export { BundleBudgetPlugin, BundleBudgetPluginOptions } from './bundle-budget';
export { ScriptsWebpackPlugin, ScriptsWebpackPluginOptions } from './scripts-webpack-plugin';
export { SuppressExtractedTextChunksWebpackPlugin } from './suppress-entry-chunks-webpack-plugin';
export { RemoveHashPlugin, RemoveHashPluginOptions } from './remove-hash-plugin';
Expand Down
92 changes: 31 additions & 61 deletions tests/legacy-cli/e2e/tests/build/bundle-budgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,70 +10,40 @@ import { ng } from '../../utils/process';
import { updateJsonFile } from '../../utils/project';
import { expectToFail } from '../../utils/utils';

// tslint:disable:max-line-length
export default function () {
const budgetConfigs = [
{
expectation: 'pass',
message: 'BIG max for all, should not error',
budget: { type: 'allScript', maximumError: '100mb' },
},
{
expectation: 'error',
message: 'Budget error: all, max error',
budget: { type: 'all', maximumError: '100b' },
},
{
expectation: 'warning',
message: 'Budget warning: all, min warning',
budget: { type: 'all', minimumWarning: '100mb' },
},
];
export default async function () {
// Error
await updateJsonFile('angular.json', json => {
json.projects['test-project'].architect.build.options.budgets = [
{ type: 'all', maximumError: '100b' },
];
});

const errorMessage = await expectToFail(() => ng('build'));
if (!/Error.+budget/.test(errorMessage)) {
throw new Error('Budget error: all, max error.');
}

const promiseFactories = budgetConfigs.map(cfg => {
if (cfg.expectation === 'error') {
return () => {
return updateJsonFile('angular.json', (json) => {
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
})
.then(() => expectToFail(() => ng('build', '--optimization')))
.then(errorMessage => {
if (!/Error.+budgets/.test(errorMessage)) {
throw new Error(cfg.message);
}
});
};
} else if (cfg.expectation === 'warning') {
return () => {
return updateJsonFile('angular.json', (json) => {
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
})
.then(() => ng('build', '--optimization'))
.then(({ stderr }) => {
if (!/Warning.+budgets/.test(stderr)) {
throw new Error(cfg.message);
}
});
};
} else { // pass
return () => {
return updateJsonFile('angular.json', (json) => {
json.projects['test-project'].architect.build.options.budgets = [cfg.budget];
})
.then(() => ng('build', '--optimization'))
.then(({ stderr }) => {
if (/(Warning|Error)/.test(stderr)) {
throw new Error(cfg.message);
}
});
};
}
// Warning
await updateJsonFile('angular.json', json => {
json.projects['test-project'].architect.build.options.budgets = [
{ type: 'all', minimumWarning: '100mb' },
];
});

let promiseChain = Promise.resolve();
for (let i = 0; i < promiseFactories.length; i++) {
promiseChain = promiseChain.then(promiseFactories[i]);
const { stderr } = await ng('build');
if (!/Warning.+budget/.test(stderr)) {
throw new Error('Budget warning: all, min warning');
}

return promiseChain;
// Pass
await updateJsonFile('angular.json', json => {
json.projects['test-project'].architect.build.options.budgets = [
{ type: 'allScript', maximumError: '100mb' },
];
});

const { stderr: stderr2 } = await ng('build');
if (/(Warning|Error)/.test(stderr2)) {
throw new Error('BIG max for all, should not error');
}
}