Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): avoid CommonJS warning for zone.j…
Browse files Browse the repository at this point in the history
…s 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.
  • Loading branch information
clydin authored and angular-robot[bot] committed Feb 13, 2023
1 parent afcc49f commit 421417a
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 11 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 5 additions & 1 deletion packages/angular_devkit/build_angular/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ LARGE_SPECS = {
"@npm//popper.js",
],
},
"browser-esbuild": {},
"browser-esbuild": {
"extra_deps": [
"@npm//buffer",
],
},
}

[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<logging.LogEntry>({
message: jasmine.stringMatching(
/Module 'buffer' used by 'src\/app\/app\.component\.ts' is not ESM/,
),
}),
);
expect(logs).toContain(
jasmine.objectContaining<logging.LogEntry>({
message: jasmine.stringMatching(/CommonJS or AMD dependencies/),
}),
);
expect(logs).not.toContain(
jasmine.objectContaining<logging.LogEntry>({
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<logging.LogEntry>({
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<logging.LogEntry>({
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<logging.LogEntry>({
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<logging.LogEntry>({
message: jasmine.stringMatching(/CommonJS or AMD dependencies/),
}),
);
});
});
});
18 changes: 8 additions & 10 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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==

[email protected], 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"
Expand All @@ -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"
Expand Down

0 comments on commit 421417a

Please sign in to comment.