From 421417a36b13a44d39e0818171482871ea8b895f Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 10 Feb 2023 20:23:56 -0500 Subject: [PATCH] fix(@angular-devkit/build-angular): avoid CommonJS warning for zone.js in esbuild The `zone.js` package is currently built into a module structure form that resembles UMD-like output. This causes the CommonJS checker within the experimental esbuild-based browser application builder to issue a warning for `zone.js` usage. Until the packaging of `zone.js` is updated to become fully ESM, the `zone.js` package is automatically allowed when performing the CommonJS module check. --- package.json | 1 + .../angular_devkit/build_angular/BUILD.bazel | 6 +- .../browser-esbuild/commonjs-checker.ts | 4 + .../allowed-common-js-dependencies_spec.ts | 163 ++++++++++++++++++ yarn.lock | 18 +- 5 files changed, 181 insertions(+), 11 deletions(-) create mode 100644 packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts diff --git a/package.json b/package.json index c80b0eb2b904..6c72a135fe75 100644 --- a/package.json +++ b/package.json @@ -136,6 +136,7 @@ "babel-plugin-istanbul": "6.1.1", "bootstrap": "^4.0.0", "browserslist": "4.21.5", + "buffer": "6.0.3", "cacache": "17.0.4", "chokidar": "3.5.3", "copy-webpack-plugin": "11.0.0", diff --git a/packages/angular_devkit/build_angular/BUILD.bazel b/packages/angular_devkit/build_angular/BUILD.bazel index 07b83d5b8c9a..f534360f121b 100644 --- a/packages/angular_devkit/build_angular/BUILD.bazel +++ b/packages/angular_devkit/build_angular/BUILD.bazel @@ -339,7 +339,11 @@ LARGE_SPECS = { "@npm//popper.js", ], }, - "browser-esbuild": {}, + "browser-esbuild": { + "extra_deps": [ + "@npm//buffer", + ], + }, } [ diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts index 2c1432f64b66..1e1591825393 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts @@ -37,6 +37,10 @@ export function checkCommonJSModules( // Ignore Angular locale definitions which are currently UMD allowedRequests.add('@angular/common/locales'); + // Ignore zone.js due to it currently being built with a UMD like structure. + // Once the build output is updated to be fully ESM, this can be removed. + allowedRequests.add('zone.js'); + // Find all entry points that contain code (JS/TS) const files: string[] = []; for (const { entryPoint } of Object.values(metafile.outputs)) { diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts new file mode 100644 index 000000000000..1270889744f3 --- /dev/null +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts @@ -0,0 +1,163 @@ +/** + * @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 { logging } from '@angular-devkit/core'; +import { buildEsbuildBrowser } from '../../index'; +import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; + +describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { + describe('Option: "allowedCommonJsDependencies"', () => { + describe('given option is not set', () => { + for (const aot of [true, false]) { + it(`should show warning when depending on a Common JS bundle in ${ + aot ? 'AOT' : 'JIT' + } Mode`, async () => { + // Add a Common JS dependency + await harness.appendToFile('src/app/app.component.ts', `import 'buffer';`); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: [], + optimization: true, + aot, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching( + /Module 'buffer' used by 'src\/app\/app\.component\.ts' is not ESM/, + ), + }), + ); + expect(logs).toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching('base64-js'), + }), + 'Should not warn on transitive CommonJS packages which parent is also CommonJS.', + ); + }); + } + }); + + it('should not show warning when depending on a Common JS bundle which is allowed', async () => { + // Add a Common JS dependency + await harness.appendToFile( + 'src/app/app.component.ts', + ` + import 'buffer'; + `, + ); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: ['buffer', 'base64-js', 'ieee754'], + optimization: true, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + }); + + it('should not show warning when depending on zone.js', async () => { + // Add a Common JS dependency + await harness.appendToFile( + 'src/app/app.component.ts', + ` + import 'zone.js'; + `, + ); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: [], + optimization: true, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + }); + + it(`should not show warning when importing non global local data '@angular/common/locale/fr'`, async () => { + await harness.appendToFile( + 'src/app/app.component.ts', + `import '@angular/common/locales/fr';`, + ); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: [], + optimization: true, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + }); + + it('should not show warning in JIT for templateUrl and styleUrl when using paths', async () => { + await harness.modifyFile('tsconfig.json', (content) => { + return content.replace( + /"baseUrl": ".\/",/, + ` + "baseUrl": "./", + "paths": { + "@app/*": [ + "src/app/*" + ] + }, + `, + ); + }); + + await harness.modifyFile('src/app/app.module.ts', (content) => + content.replace('./app.component', '@app/app.component'), + ); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: [], + optimization: true, + aot: false, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 9fe3dbe47255..bbcbd2f873ae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -121,7 +121,6 @@ "@angular/build-tooling@https://github.com/angular/dev-infra-private-build-tooling-builds.git#f7d26a0b0d6bd2043f2d32c2a99db903539d0c07": version "0.0.0-07b0f6423e0c5266b3792d8f4af43b8fd3f3d41b" - uid f7d26a0b0d6bd2043f2d32c2a99db903539d0c07 resolved "https://github.com/angular/dev-infra-private-build-tooling-builds.git#f7d26a0b0d6bd2043f2d32c2a99db903539d0c07" dependencies: "@angular-devkit/build-angular" "15.2.0-next.3" @@ -307,7 +306,6 @@ "@angular/ng-dev@https://github.com/angular/dev-infra-private-ng-dev-builds.git#8aa60413b3e14daf2f33a29fe9d09faa3e5bcb75": version "0.0.0-07b0f6423e0c5266b3792d8f4af43b8fd3f3d41b" - uid "8aa60413b3e14daf2f33a29fe9d09faa3e5bcb75" resolved "https://github.com/angular/dev-infra-private-ng-dev-builds.git#8aa60413b3e14daf2f33a29fe9d09faa3e5bcb75" dependencies: "@yarnpkg/lockfile" "^1.1.0" @@ -4499,6 +4497,14 @@ buffer-from@^1.0.0: resolved "https://registry.yarnpkg.com/buffer-from/-/buffer-from-1.1.2.tgz#2b146a6fd72e80b4f55d255f35ed59a3a9a41bd5" integrity sha512-E+XQCRwSbaaiChtv6k6Dwgc+bx+Bs6vuKJHHl5kox/BaKbhiXzqQOwK4cO22yElGp2OCmjwVhT3HmxgyPGnJfQ== +buffer@6.0.3, buffer@^6.0.3: + version "6.0.3" + resolved "https://registry.yarnpkg.com/buffer/-/buffer-6.0.3.tgz#2ace578459cc8fbe2a70aaa8f52ee63b6a74c6c6" + integrity sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA== + dependencies: + base64-js "^1.3.1" + ieee754 "^1.2.1" + buffer@^5.2.1, buffer@^5.5.0: version "5.7.1" resolved "https://registry.yarnpkg.com/buffer/-/buffer-5.7.1.tgz#ba62e7c13133053582197160851a8f648e99eed0" @@ -4507,14 +4513,6 @@ buffer@^5.2.1, buffer@^5.5.0: base64-js "^1.3.1" ieee754 "^1.1.13" -buffer@^6.0.3: - version "6.0.3" - resolved "https://registry.yarnpkg.com/buffer/-/buffer-6.0.3.tgz#2ace578459cc8fbe2a70aaa8f52ee63b6a74c6c6" - integrity sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA== - dependencies: - base64-js "^1.3.1" - ieee754 "^1.2.1" - builtin-modules@^3.3.0: version "3.3.0" resolved "https://registry.yarnpkg.com/builtin-modules/-/builtin-modules-3.3.0.tgz#cae62812b89801e9656336e46223e030386be7b6"