From 9c13fce162eff8d01d1fa6a7f0e0029da2887c86 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 23 Sep 2022 18:53:41 +0000 Subject: [PATCH] feat(@angular-devkit/build-angular): remove `bundleDependencies` from server builder This commit removes the usages of `bundleDependencies` which does not correctly work as webpack will use `require` to import ESM module since we configure the server bundle to be outputted in CJS. Migrating fully to ESM is also currently not viable due to the lack of support from Domino. Even if full ESM was possible, using this option would have resulted in a runtime overhead as Angular libraries would be linked during runtime instead of compile time. BREAKING CHANGE: The server builder `bundleDependencies` option has been removed. This option was used pre Ivy. Currently, using this option is unlikely to produce working server bundles. The `externalDependencies` option can be used instead to exclude specific node_module packages from the final bundle. Closes #23905 --- .../angular_devkit/build_angular/index.md | 1 - .../src/builders/server/schema.json | 5 -- .../build_angular/src/utils/build-options.ts | 1 - .../src/webpack/configs/common.ts | 12 +-- .../src/webpack/utils/helpers.ts | 25 ------ .../migrations/migration-collection.json | 5 ++ .../update-15/update-workspace-config.ts | 27 ++++++ .../update-15/update-workspace-config_spec.ts | 86 +++++++++++++++++++ .../e2e/tests/build/platform-server.ts | 4 +- .../misc/universal-bundle-dependencies.ts | 66 -------------- 10 files changed, 121 insertions(+), 111 deletions(-) create mode 100644 packages/schematics/angular/migrations/update-15/update-workspace-config.ts create mode 100644 packages/schematics/angular/migrations/update-15/update-workspace-config_spec.ts delete mode 100644 tests/legacy-cli/e2e/tests/misc/universal-bundle-dependencies.ts diff --git a/goldens/public-api/angular_devkit/build_angular/index.md b/goldens/public-api/angular_devkit/build_angular/index.md index e47d0218f21c..a8f748bc8504 100644 --- a/goldens/public-api/angular_devkit/build_angular/index.md +++ b/goldens/public-api/angular_devkit/build_angular/index.md @@ -244,7 +244,6 @@ export interface ProtractorBuilderOptions { // @public (undocumented) export interface ServerBuilderOptions { - bundleDependencies?: boolean; deleteOutputPath?: boolean; // @deprecated deployUrl?: string; diff --git a/packages/angular_devkit/build_angular/src/builders/server/schema.json b/packages/angular_devkit/build_angular/src/builders/server/schema.json index f3ce0c52f0cf..283d25363f37 100644 --- a/packages/angular_devkit/build_angular/src/builders/server/schema.json +++ b/packages/angular_devkit/build_angular/src/builders/server/schema.json @@ -181,11 +181,6 @@ "description": "Use file name for lazy loaded chunks.", "default": false }, - "bundleDependencies": { - "description": "Which external dependencies to bundle into the bundle. By default, all of node_modules will be bundled.", - "default": true, - "type": "boolean" - }, "externalDependencies": { "description": "Exclude the listed external dependencies from being bundled into the bundle. Instead, the created bundle relies on these dependencies to be available during runtime.", "type": "array", diff --git a/packages/angular_devkit/build_angular/src/utils/build-options.ts b/packages/angular_devkit/build_angular/src/utils/build-options.ts index a986b9786415..49e1212bd2c3 100644 --- a/packages/angular_devkit/build_angular/src/utils/build-options.ts +++ b/packages/angular_devkit/build_angular/src/utils/build-options.ts @@ -41,7 +41,6 @@ export interface BuildOptions { progress?: boolean; localize?: Localize; i18nMissingTranslation?: I18NTranslation; - bundleDependencies?: boolean; externalDependencies?: string[]; watch?: boolean; outputHashing?: OutputHashing; diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts index be04758a5bc6..7cbfe39404ec 100644 --- a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts +++ b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts @@ -9,7 +9,6 @@ import { AngularWebpackLoaderPath } from '@ngtools/webpack'; import CopyWebpackPlugin from 'copy-webpack-plugin'; import * as path from 'path'; -import { ScriptTarget } from 'typescript'; import { Compiler, Configuration, @@ -37,7 +36,6 @@ import { createIvyPlugin } from '../plugins/typescript'; import { WatchFilesLogsPlugin } from '../plugins/watch-files-logs-plugin'; import { assetPatterns, - externalizePackages, getCacheSettings, getInstrumentationExcludedPaths, getOutputHashFormat, @@ -74,7 +72,6 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise - externalizePackages(context ?? wco.projectRoot, request, callback), - ); - } - let crossOriginLoading: NonNullable['crossOriginLoading'] = false; if (subresourceIntegrity && crossOrigin === 'none') { crossOriginLoading = 'anonymous'; @@ -307,7 +297,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise void, -): void { - if (!request) { - return; - } - - // Absolute & Relative paths are not externals - if (request.startsWith('.') || path.isAbsolute(request)) { - callback(); - - return; - } - - try { - require.resolve(request, { paths: [context] }); - callback(undefined, request); - } catch { - // Node couldn't find it, so it must be user-aliased - callback(); - } -} - export function getStatsOptions(verbose = false): WebpackStatsOptions { const webpackOutputOptions: WebpackStatsOptions = { all: false, // Fallback value for stats options when an option is not defined. It has precedence over local webpack defaults. diff --git a/packages/schematics/angular/migrations/migration-collection.json b/packages/schematics/angular/migrations/migration-collection.json index a19748126b49..d402927d5f6b 100644 --- a/packages/schematics/angular/migrations/migration-collection.json +++ b/packages/schematics/angular/migrations/migration-collection.json @@ -14,6 +14,11 @@ "version": "15.0.0", "factory": "./update-15/update-typescript-target", "description": "Update TypeScript compiler `target` and set `useDefineForClassFields`. These changes are for IDE purposes as TypeScript compiler options `target` and `useDefineForClassFields` are set to `ES2022` and `false` respectively by the Angular CLI. To control ECMA version and features use the Browerslist configuration." + }, + "update-workspace-config": { + "version": "15.0.0", + "factory": "./update-15/update-workspace-config", + "description": "Remove options from 'angular.json' that are no longer supported by the official builders." } } } diff --git a/packages/schematics/angular/migrations/update-15/update-workspace-config.ts b/packages/schematics/angular/migrations/update-15/update-workspace-config.ts new file mode 100644 index 000000000000..2b8686e8cf5a --- /dev/null +++ b/packages/schematics/angular/migrations/update-15/update-workspace-config.ts @@ -0,0 +1,27 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import { Rule } from '@angular-devkit/schematics'; +import { allTargetOptions, updateWorkspace } from '../../utility/workspace'; +import { Builders } from '../../utility/workspace-models'; + +export default function (): Rule { + return updateWorkspace((workspace) => { + for (const project of workspace.projects.values()) { + for (const target of project.targets.values()) { + if (target.builder !== Builders.Server) { + continue; + } + + for (const [, options] of allTargetOptions(target)) { + delete options.bundleDependencies; + } + } + } + }); +} diff --git a/packages/schematics/angular/migrations/update-15/update-workspace-config_spec.ts b/packages/schematics/angular/migrations/update-15/update-workspace-config_spec.ts new file mode 100644 index 000000000000..553ad9ef4d43 --- /dev/null +++ b/packages/schematics/angular/migrations/update-15/update-workspace-config_spec.ts @@ -0,0 +1,86 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import { JsonObject } from '@angular-devkit/core'; +import { EmptyTree } from '@angular-devkit/schematics'; +import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing'; +import { + BuilderTarget, + Builders, + ProjectType, + WorkspaceSchema, +} from '../../utility/workspace-models'; + +function getServerTarget(tree: UnitTestTree): BuilderTarget { + const target = (tree.readJson('/angular.json') as unknown as WorkspaceSchema).projects.app + .architect?.server; + + return target as unknown as BuilderTarget; +} + +function createWorkSpaceConfig(tree: UnitTestTree) { + const angularConfig: WorkspaceSchema = { + version: 1, + projects: { + app: { + root: '', + sourceRoot: 'src', + projectType: ProjectType.Application, + prefix: 'app', + architect: { + server: { + builder: Builders.Server, + options: { + main: './server.ts', + bundleDependencies: false, + sourceMaps: true, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + configurations: { + one: { + aot: true, + }, + two: { + bundleDependencies: true, + aot: true, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + }, + }, + }, + }, + }; + + tree.create('/angular.json', JSON.stringify(angularConfig, undefined, 2)); +} + +const schematicName = 'update-workspace-config'; + +describe(`Migration to update 'angular.json'. ${schematicName}`, () => { + const schematicRunner = new SchematicTestRunner( + 'migrations', + require.resolve('../migration-collection.json'), + ); + + let tree: UnitTestTree; + beforeEach(() => { + tree = new UnitTestTree(new EmptyTree()); + createWorkSpaceConfig(tree); + }); + + it(`should remove 'bundleDependencies'`, async () => { + const newTree = await schematicRunner.runSchematicAsync(schematicName, {}, tree).toPromise(); + const { options, configurations } = getServerTarget(newTree); + + expect(options.bundleDependencies).toBeUndefined(); + expect(configurations).toBeDefined(); + expect(configurations?.one.bundleDependencies).toBeUndefined(); + expect(configurations?.two.bundleDependencies).toBeUndefined(); + }); +}); diff --git a/tests/legacy-cli/e2e/tests/build/platform-server.ts b/tests/legacy-cli/e2e/tests/build/platform-server.ts index dd10b7bafd80..7dedb5e06966 100644 --- a/tests/legacy-cli/e2e/tests/build/platform-server.ts +++ b/tests/legacy-cli/e2e/tests/build/platform-server.ts @@ -59,8 +59,8 @@ export default async function () { /Here are some links to help you get started:<\/p>/, ); - // works with optimization and bundleDependencies enabled - await ng('run', 'test-project:server', '--optimization', '--bundle-dependencies'); + // works with optimization + await ng('run', 'test-project:server', '--optimization'); await exec(normalize('node'), 'dist/test-project/server/main.js'); await expectFileToMatch( 'dist/test-project/server/index.html', diff --git a/tests/legacy-cli/e2e/tests/misc/universal-bundle-dependencies.ts b/tests/legacy-cli/e2e/tests/misc/universal-bundle-dependencies.ts deleted file mode 100644 index 570179b83456..000000000000 --- a/tests/legacy-cli/e2e/tests/misc/universal-bundle-dependencies.ts +++ /dev/null @@ -1,66 +0,0 @@ -import * as path from 'path'; -import { - createDir, - expectFileToMatch, - rimraf, - symlinkFile, - writeMultipleFiles, -} from '../../utils/fs'; -import { ng } from '../../utils/process'; -import { updateJsonFile } from '../../utils/project'; - -export default async function () { - await updateJsonFile('angular.json', (workspaceJson) => { - const appArchitect = workspaceJson.projects['test-project'].architect; - appArchitect['server'] = { - builder: '@angular-devkit/build-angular:server', - options: { - bundleDependencies: false, - outputPath: 'dist/test-project-server', - main: 'src/main.server.ts', - tsConfig: 'tsconfig.server.json', - }, - }; - }); - - await createDir('./dummy-lib'); - - await writeMultipleFiles({ - './tsconfig.server.json': ` - { - "extends": "./tsconfig.json", - "compilerOptions": { - "outDir": "../dist-server", - "baseUrl": "./", - "module": "commonjs", - "types": [] - }, - "include": [ - "src/main.server.ts" - ] - } - `, - './src/main.server.ts': ` - import { dummyVersion } from 'dummy-lib'; - console.log(dummyVersion); - `, - // create a dummy library - './dummy-lib/package.json': `{ - "name": "dummy-lib", - "version": "0.0.0", - "typings": "./main.d.ts", - "main": "./main.js" - }`, - './dummy-lib/main.js': 'export const dummyVersion = 1', - './dummy-lib/main.d.ts': 'export declare const dummyVersion = 1', - }); - - await symlinkFile(path.resolve('./dummy-lib'), path.resolve('./node_modules/dummy-lib'), 'dir'); - - await ng('run', 'test-project:server'); - // when preserve symlinks is true, it should not included node_modules in the bundle - await expectFileToMatch('dist/test-project-server/main.js', 'require("dummy-lib")'); - - // cleanup the package - await rimraf('node_modules/dummy-lib'); -}