Skip to content

Commit

Permalink
fix(@angular/build): issue warning when auto adding `@angular/localiz…
Browse files Browse the repository at this point in the history
…e/init`

This commit introduces a warning for when the application builders automatically add the @angular/localize/init polyfill. The current approach has a drawback: the localize polyfill will always be included if it is found in a monorepo, even if an application does not use i18n.

To address this, we will issue a warning to inform users about this behavior and encourage them to explicitly add the polyfill to their polyfills configuration.

Additionally, this commit fixes an issue where the polyfill was not removed when using the build-time Angular i18n.

(cherry picked from commit fbc6eb3)
  • Loading branch information
alan-agius4 committed Jun 7, 2024
1 parent a21b582 commit 78c6117
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import type { BuildOptions } from 'esbuild';
import type { BuildOptions, PartialMessage } from 'esbuild';
import assert from 'node:assert';
import { createHash } from 'node:crypto';
import { readFile } from 'node:fs/promises';
Expand Down Expand Up @@ -403,7 +403,7 @@ function getEsBuildCommonPolyfillsOptions(
plugins: [createSourcemapIgnorelistPlugin()],
};

const polyfills = options.polyfills ? [...options.polyfills] : [];
let polyfills = options.polyfills ? [...options.polyfills] : [];

// Angular JIT mode requires the runtime compiler
if (jit) {
Expand All @@ -414,6 +414,9 @@ function getEsBuildCommonPolyfillsOptions(
// Locale data should go first so that project provided polyfill code can augment if needed.
let needLocaleDataPlugin = false;
if (i18nOptions.shouldInline) {
// Remove localize polyfill as this is not needed for build time i18n.
polyfills = polyfills.filter((path) => !path.startsWith('@angular/localize'));

// Add locale data for all active locales
// TODO: Inject each individually within the inlining process itself
for (const locale of i18nOptions.inlineLocales) {
Expand All @@ -440,6 +443,7 @@ function getEsBuildCommonPolyfillsOptions(
loadContent: async (_, build) => {
let hasLocalizePolyfill = false;
let polyfillPaths = polyfills;
let warnings: PartialMessage[] | undefined;

if (tryToResolvePolyfillsAsRelative) {
polyfillPaths = await Promise.all(
Expand All @@ -462,6 +466,8 @@ function getEsBuildCommonPolyfillsOptions(
hasLocalizePolyfill = polyfills.some((p) => p.startsWith('@angular/localize'));
}

// Add localize polyfill if needed.
// TODO: remove in version 19 or later.
if (!i18nOptions.shouldInline && !hasLocalizePolyfill) {
const result = await build.resolve('@angular/localize', {
kind: 'import-statement',
Expand All @@ -470,6 +476,17 @@ function getEsBuildCommonPolyfillsOptions(

if (result.path) {
polyfillPaths.push('@angular/localize/init');

(warnings ??= []).push({
text: 'Polyfill for "@angular/localize/init" was added automatically.',
notes: [
{
text:
'In the future, this functionality will be removed. ' +
'Please add this polyfill in the "polyfills" section of your "angular.json" instead.',
},
],
});
}
}

Expand All @@ -490,6 +507,7 @@ function getEsBuildCommonPolyfillsOptions(
return {
contents,
loader: 'js',
warnings,
resolveDir: workspaceRoot,
};
},
Expand Down
3 changes: 2 additions & 1 deletion tests/legacy-cli/e2e/tests/i18n/extract-ivy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export default async function () {
// Should not show any warnings when extracting
const { stderr: message5 } = await ng('extract-i18n');
if (message5.includes('WARNING')) {
throw new Error('Expected no warnings to be shown. STDERR:\n' + message5);
// TODO: enable once https://github.com/angular/angular/pull/56300 is released.
// throw new Error('Expected no warnings to be shown. STDERR:\n' + message5);
}

await expectFileToMatch('messages.xlf', 'Hello world');
Expand Down

0 comments on commit 78c6117

Please sign in to comment.