Skip to content

fix(@angular-devkit/build-angular): remove non-global locale import warning #18134

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.

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
merged 1 commit into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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();
});
});