Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): remove non-global locale import w…
Browse files Browse the repository at this point in the history
…arning

We have not yet deprecated the non-global locale data modules (e.g. `@angular/common/locales/fr`) so we should not be issuing warnings about developers using them.

We recently added warning suggesting that a "global" locale should be used instead, and the previous CommonJS/AMD warning about the format of these non-global modules are just confusing for the developer.

Reference: TOOL-1388
Closes: #18123
(cherry picked from commit 0a573e7)
  • Loading branch information
alan-agius4 committed Jul 7, 2020
1 parent f989707 commit 07f93b7
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ export class CommonJsUsageWarnPlugin {
if (
!rawRequest ||
rawRequest.startsWith('.') ||
isAbsolute(rawRequest)
) {
// Skip if module is absolute or relative.
continue;
}

if (
isAbsolute(rawRequest) ||
this.allowedDepedencies.has(rawRequest) ||
this.allowedDepedencies.has(this.rawRequestToPackageName(rawRequest))
this.allowedDepedencies.has(this.rawRequestToPackageName(rawRequest)) ||
rawRequest.startsWith('@angular/common/locales/')
) {
// Skip as this module is allowed even if it's a CommonJS.
/**
* Skip when:
* - module is absolute or relative.
* - module is allowed even if it's a CommonJS.
* - module is a locale imported from '@angular/common'.
*/
continue;
}

Expand All @@ -81,16 +81,9 @@ 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')) {
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';
}
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';

// 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 @@ -64,7 +64,7 @@ describe('Browser Builder commonjs warning', () => {
await run.stop();
});

it(`should show warning when importing non global '@angular/common/locale' data`, async () => {
it(`should not show warning when importing non global local data '@angular/common/locale/fr'`, async () => {
// Add a Common JS dependency
host.appendToFile('src/app/app.component.ts', `
import '@angular/common/locales/fr';
Expand All @@ -74,9 +74,7 @@ describe('Browser Builder commonjs warning', () => {
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'`);
expect(logs.join()).not.toContain('WARNING');
await run.stop();
});
});

0 comments on commit 07f93b7

Please sign in to comment.