From 411786a4cd3fc63bae786a8da422d2a87fa69aaf Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 6 Aug 2021 12:39:16 -0400 Subject: [PATCH 1/2] refactor(@angular-devkit/build-angular): remove unneeded ivy enabled checks Applications can now only be built with Ivy. However, several checks were still present in the code to determine if Ivy was enabled. Since Ivy is always enabled these checks have since become unused code and can be removed. --- .../src/builders/dev-server/index.ts | 17 ++++++---------- .../src/builders/extract-i18n/index.ts | 6 ------ .../src/builders/server/index.ts | 2 +- .../src/webpack/configs/analytics.ts | 9 +-------- .../legacy-cli/e2e/tests/i18n/extract-ivy.ts | 20 ++----------------- 5 files changed, 10 insertions(+), 44 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/dev-server/index.ts b/packages/angular_devkit/build_angular/src/builders/dev-server/index.ts index 683d7bb6ce27..a352e6299898 100644 --- a/packages/angular_devkit/build_angular/src/builders/dev-server/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/dev-server/index.ts @@ -27,7 +27,6 @@ import { colors } from '../../utils/color'; import { I18nOptions } from '../../utils/i18n-options'; import { IndexHtmlTransform } from '../../utils/index-file/index-html-generator'; import { generateEntryPoints } from '../../utils/package-chunk-sort'; -import { readTsconfig } from '../../utils/read-tsconfig'; import { assertCompatibleAngularVersion } from '../../utils/version'; import { generateI18nBrowserWebpackConfigFromContext, @@ -276,17 +275,13 @@ export function serveWebpackBrowser( // If a locale is defined, setup localization if (locale) { - // Only supported with Ivy - const tsConfig = readTsconfig(browserOptions.tsConfig, workspaceRoot); - if (tsConfig.options.enableIvy !== false) { - if (i18n.inlineLocales.size > 1) { - throw new Error( - 'The development server only supports localizing a single locale per build.', - ); - } - - await setupLocalize(locale, i18n, browserOptions, webpackConfig); + if (i18n.inlineLocales.size > 1) { + throw new Error( + 'The development server only supports localizing a single locale per build.', + ); } + + await setupLocalize(locale, i18n, browserOptions, webpackConfig); } if (transforms.webpackConfiguration) { diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts index 3b3e7ab085c3..a82ada953fc2 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts @@ -200,12 +200,6 @@ export async function execute( builderOptions, context, (wco) => { - if (wco.tsConfig.options.enableIvy === false) { - context.logger.warn( - 'Ivy extraction enabled but application is not Ivy enabled. Extraction may fail.', - ); - } - // Default value for legacy message ids is currently true useLegacyIds = wco.tsConfig.options.enableI18nLegacyMessageIdFormat ?? true; diff --git a/packages/angular_devkit/build_angular/src/builders/server/index.ts b/packages/angular_devkit/build_angular/src/builders/server/index.ts index ea60869d840f..efffbb4673dd 100644 --- a/packages/angular_devkit/build_angular/src/builders/server/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/server/index.ts @@ -74,7 +74,7 @@ export function execute( ); } - if (!options.bundleDependencies && tsConfig.options.enableIvy) { + if (!options.bundleDependencies) { // eslint-disable-next-line import/no-extraneous-dependencies const { __processed_by_ivy_ngcc__, main = '' } = require('@angular/core/package.json'); if ( diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/analytics.ts b/packages/angular_devkit/build_angular/src/webpack/configs/analytics.ts index 38996af2ee67..e640c5221a8c 100644 --- a/packages/angular_devkit/build_angular/src/webpack/configs/analytics.ts +++ b/packages/angular_devkit/build_angular/src/webpack/configs/analytics.ts @@ -28,13 +28,6 @@ export function getAnalyticsConfig( // The category is the builder name if it's an angular builder. return { - plugins: [ - new NgBuildAnalyticsPlugin( - wco.projectRoot, - context.analytics, - category, - !!wco.tsConfig.options.enableIvy, - ), - ], + plugins: [new NgBuildAnalyticsPlugin(wco.projectRoot, context.analytics, category, true)], }; } diff --git a/tests/legacy-cli/e2e/tests/i18n/extract-ivy.ts b/tests/legacy-cli/e2e/tests/i18n/extract-ivy.ts index a64eb2604e0f..8dde1f1bdfd1 100644 --- a/tests/legacy-cli/e2e/tests/i18n/extract-ivy.ts +++ b/tests/legacy-cli/e2e/tests/i18n/extract-ivy.ts @@ -7,13 +7,10 @@ import { updateJsonFile } from '../../utils/project'; import { expectToFail } from '../../utils/utils'; import { readNgVersion } from '../../utils/version'; -export default async function() { +export default async function () { // Setup an i18n enabled component await ng('generate', 'component', 'i18n-test'); - await writeFile( - join('src/app/i18n-test', 'i18n-test.component.html'), - '

Hello world

', - ); + await writeFile(join('src/app/i18n-test', 'i18n-test.component.html'), '

Hello world

'); // Should fail if `@angular/localize` is missing const { message: message1 } = await expectToFail(() => ng('extract-i18n')); @@ -34,18 +31,5 @@ export default async function() { throw new Error('Expected no warnings to be shown'); } - // Disable Ivy - await updateJsonFile('tsconfig.json', config => { - const { angularCompilerOptions = {} } = config; - angularCompilerOptions.enableIvy = false; - config.angularCompilerOptions = angularCompilerOptions; - }); - - // Should show ivy disabled application warning with enableIvy false - const { stderr: message4 } = await ng('extract-i18n'); - if (!message4.includes(`Ivy extraction enabled but application is not Ivy enabled.`)) { - throw new Error('Expected ivy disabled application warning'); - } - await uninstallPackage('@angular/localize'); } From bbcdf5fcca763209bc86e363386a104e072ec3ff Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 6 Aug 2021 12:45:50 -0400 Subject: [PATCH 2/2] refactor(@angular-devkit/build-angular): reduce repeat TypeScript configuration loading An application's TypeScript configuration was previously being loaded multiple times in several different aspects of the build setup process. These aspects need to access specific compiler options relevant to that particular area of the setup. However, loading the configuration can be expensive due to the process also calculating the root files for the TypeScript compilation which can result in a large amount of file access. To improve the setup performance, the number of times the TypeScript configuration will be loaded has now been reduced with further reductions possible with additional refactorings. --- .../build_angular/src/builders/browser/index.ts | 13 ++++--------- .../build_angular/src/builders/server/index.ts | 10 ++++------ .../src/utils/webpack-browser-config.ts | 11 +++++++++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/browser/index.ts b/packages/angular_devkit/build_angular/src/builders/browser/index.ts index c0dc9ce147b3..4c86673fcfb5 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser/index.ts @@ -35,7 +35,6 @@ import { } from '../../utils/index-file/index-html-generator'; import { ensureOutputPaths } from '../../utils/output-paths'; import { generateEntryPoints } from '../../utils/package-chunk-sort'; -import { readTsconfig } from '../../utils/read-tsconfig'; import { augmentAppWithServiceWorker } from '../../utils/service-worker'; import { Spinner } from '../../utils/spinner'; import { assertCompatibleAngularVersion } from '../../utils/version'; @@ -86,13 +85,14 @@ async function initialize( projectRoot: string; projectSourceRoot?: string; i18n: I18nOptions; + target: ScriptTarget; }> { const originalOutputPath = options.outputPath; // Assets are processed directly by the builder except when watching const adjustedOptions = options.watch ? options : { ...options, assets: [] }; - const { config, projectRoot, projectSourceRoot, i18n } = + const { config, projectRoot, projectSourceRoot, i18n, target } = await generateI18nBrowserWebpackConfigFromContext(adjustedOptions, context, (wco) => [ getCommonConfig(wco), getBrowserConfig(wco), @@ -126,7 +126,7 @@ async function initialize( deleteOutputDir(context.workspaceRoot, originalOutputPath); } - return { config: transformedConfig || config, projectRoot, projectSourceRoot, i18n }; + return { config: transformedConfig || config, projectRoot, projectSourceRoot, i18n, target }; } /** @@ -164,16 +164,11 @@ export function buildWebpackBrowser( ), ); - const { options: compilerOptions } = readTsconfig(options.tsConfig, context.workspaceRoot); - const target = compilerOptions.target || ScriptTarget.ES5; const buildBrowserFeatures = new BuildBrowserFeatures(sysProjectRoot); checkInternetExplorerSupport(buildBrowserFeatures.supportedBrowsers, context.logger); - return { - ...(await initialize(options, context, transforms.webpackConfiguration)), - target, - }; + return initialize(options, context, transforms.webpackConfiguration); }), switchMap( // eslint-disable-next-line max-lines-per-function diff --git a/packages/angular_devkit/build_angular/src/builders/server/index.ts b/packages/angular_devkit/build_angular/src/builders/server/index.ts index efffbb4673dd..266ff9ea2eec 100644 --- a/packages/angular_devkit/build_angular/src/builders/server/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/server/index.ts @@ -19,7 +19,6 @@ import { NormalizedBrowserBuilderSchema, deleteOutputDir } from '../../utils'; import { i18nInlineEmittedFiles } from '../../utils/i18n-inlining'; import { I18nOptions } from '../../utils/i18n-options'; import { ensureOutputPaths } from '../../utils/output-paths'; -import { readTsconfig } from '../../utils/read-tsconfig'; import { assertCompatibleAngularVersion } from '../../utils/version'; import { generateI18nBrowserWebpackConfigFromContext } from '../../utils/webpack-browser-config'; import { @@ -62,8 +61,6 @@ export function execute( // Check Angular version. assertCompatibleAngularVersion(root); - const tsConfig = readTsconfig(options.tsConfig, root); - const target = tsConfig.options.target || ScriptTarget.ES5; const baseOutputPath = path.resolve(root, options.outputPath); let outputPaths: undefined | Map; @@ -91,7 +88,7 @@ export function execute( } return from(initialize(options, context, transforms.webpackConfiguration)).pipe( - concatMap(({ config, i18n }) => { + concatMap(({ config, i18n, target }) => { return runWebpack(config, context, { webpackFactory: require('webpack') as typeof webpack, logging: (stats, config) => { @@ -153,9 +150,10 @@ async function initialize( ): Promise<{ config: webpack.Configuration; i18n: I18nOptions; + target: ScriptTarget; }> { const originalOutputPath = options.outputPath; - const { config, i18n } = await generateI18nBrowserWebpackConfigFromContext( + const { config, i18n, target } = await generateI18nBrowserWebpackConfigFromContext( { ...options, buildOptimizer: false, @@ -181,5 +179,5 @@ async function initialize( deleteOutputDir(context.workspaceRoot, originalOutputPath); } - return { config: transformedConfig || config, i18n }; + return { config: transformedConfig || config, i18n, target }; } diff --git a/packages/angular_devkit/build_angular/src/utils/webpack-browser-config.ts b/packages/angular_devkit/build_angular/src/utils/webpack-browser-config.ts index 677ca500a336..4d9bcb83c624 100644 --- a/packages/angular_devkit/build_angular/src/utils/webpack-browser-config.ts +++ b/packages/angular_devkit/build_angular/src/utils/webpack-browser-config.ts @@ -9,6 +9,7 @@ import { BuilderContext } from '@angular-devkit/architect'; import { getSystemPath, logging, normalize, resolve } from '@angular-devkit/core'; import * as path from 'path'; +import { ScriptTarget } from 'typescript'; import { Configuration, javascript } from 'webpack'; import { merge as webpackMerge } from 'webpack-merge'; import { Schema as BrowserBuilderSchema } from '../builders/browser/schema'; @@ -69,12 +70,18 @@ export async function generateI18nBrowserWebpackConfigFromContext( projectRoot: string; projectSourceRoot?: string; i18n: I18nOptions; + target: ScriptTarget; }> { const { buildOptions, i18n } = await configureI18nBuild(context, options); + let target = ScriptTarget.ES5; const result = await generateBrowserWebpackConfigFromContext( buildOptions, context, - webpackPartialGenerator, + (wco) => { + target = wco.scriptTarget; + + return webpackPartialGenerator(wco); + }, extraBuildOptions, ); const config = result.config; @@ -119,7 +126,7 @@ export async function generateI18nBrowserWebpackConfigFromContext( }); } - return { ...result, i18n }; + return { ...result, i18n, target }; } export async function generateBrowserWebpackConfigFromContext( options: BrowserBuilderSchema,