Skip to content

fix(@angular-devkit/build-angular): avoid CommonJS warning for zone.js in esbuild #24699

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.

Sign up for GitHub

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

Merged
angular-robot merged 1 commit into from
Feb 13, 2023
Merged

fix(@angular-devkit/build-angular): avoid CommonJS warning for zone.js in esbuild #24699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,11 @@ LARGE_SPECS = {
"@npm//popper.js",
],
},
"browser-esbuild": {},
"browser-esbuild": {
"extra_deps": [
"@npm//buffer",
],
},
}

[
Expand Down
4 changes: 4 additions & 0 deletions packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts
View file
Open in desktop
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
163 changes: 163 additions & 0 deletions ...angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts
View file
Open in desktop
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
View file
Open in desktop
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 @@ -4389,6 +4387,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"
Expand All @@ -4397,14 +4403,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