Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): show warning when using non globa…
Browse files Browse the repository at this point in the history
…l locale data

When using the `localize` option directly importing locale data from `@angular/common` is not needed because the Angular CLI  will automatically include locale data. When not using the `localize` option, most likely users meant to import the global variant of the local data.

See: https://angular.io/guide/i18n#import-global-variants-of-the-locale-data
  • Loading branch information
alan-agius4 authored and filipesilva committed Jul 1, 2020
1 parent d74b7c9 commit 68ed9ab
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,16 @@ export class CommonJsUsageWarnPlugin {
// And if the issuer request is not from 'webpack-dev-server', as 'webpack-dev-server'
// will require CommonJS libraries for live reloading such as 'sockjs-node'.
if (mainIssuer?.name === 'main' && !issuer?.userRequest?.includes('webpack-dev-server')) {
const warning = `${issuer?.userRequest} depends on ${rawRequest}. CommonJS or AMD dependencies can cause optimization bailouts.\n` +
'For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies';
let warning = `${issuer?.userRequest} depends on '${rawRequest}'.`;

if (rawRequest.startsWith('@angular/common/locales')) {
warning += `\nWhen using the 'localize' option this import is not needed. ` +
`Did you mean to import '${rawRequest.replace(/locales(\/extra)?\//, 'locales/global/')}'?\n` +
'For more info see: https://angular.io/guide/i18n#import-global-variants-of-the-locale-data';
} else {
warning += ' CommonJS or AMD dependencies can cause optimization bailouts.\n' +
'For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies';
}

// Avoid showing the same warning multiple times when in 'watch' mode.
if (!this.shownWarnings.has(warning)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ describe('Browser Builder commonjs warning', () => {
await host.initialize().toPromise();
architect = (await createArchitect(host.root())).architect;

// Add a Common JS dependency
host.appendToFile('src/app/app.component.ts', `
import 'bootstrap';
import 'zone.js/dist/zone-error';
`);

// Create logger
logger = new logging.Logger('');
logs = [];
Expand All @@ -35,16 +29,27 @@ describe('Browser Builder commonjs warning', () => {
afterEach(async () => host.restore().toPromise());

it('should show warning when depending on a Common JS bundle', async () => {
// Add a Common JS dependency
host.appendToFile('src/app/app.component.ts', `
import 'bootstrap';
`);

const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
const output = await run.result;
expect(output.success).toBe(true);
const logMsg = logs.join();
expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on bootstrap\. CommonJS or AMD dependencies/);
expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on 'bootstrap'\. CommonJS or AMD dependencies/);
expect(logMsg).not.toContain('jquery', 'Should not warn on transitive CommonJS packages which parent is also CommonJS.');
await run.stop();
});

it('should not show warning when depending on a Common JS bundle which is allowed', async () => {
// Add a Common JS dependency
host.appendToFile('src/app/app.component.ts', `
import 'bootstrap';
import 'zone.js/dist/zone-error';
`);

const overrides = {
allowedCommonJsDependencies: [
'bootstrap',
Expand All @@ -58,4 +63,20 @@ describe('Browser Builder commonjs warning', () => {
expect(logs.join()).not.toContain('WARNING');
await run.stop();
});

it(`should show warning when importing non global '@angular/common/locale' data`, async () => {
// Add a Common JS dependency
host.appendToFile('src/app/app.component.ts', `
import '@angular/common/locales/fr';
`);

const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
const output = await run.result;
expect(output.success).toBe(true);

const logMsg = logs.join();
expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on '@angular\/common\/locales\/fr'/);
expect(logMsg).toContain(`Did you mean to import '@angular/common/locales/global/fr'`);
await run.stop();
});
});

0 comments on commit 68ed9ab

Please sign in to comment.