-
Notifications
You must be signed in to change notification settings - Fork 11.9k
fix(@angular/build): builder removes @angular/localize when external packages are not inlined #31721
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
fix(@angular/build): builder removes @angular/localize when external packages are not inlined #31721
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I'm a bit confused by the description of this change. Currently, |
|
Hi @alan-agius4 Apologies for the misleading reference, at first I thought that I'd check if there are Vite's pre-bundles dependencies (which are not inlined) by looking at the optimized script presence. Now let me clarify the issue that I've found: The i18n inliner processes Ideally, the localize script should only be removed if there are no pre-bundled dependencies containing What do you think? |
|
Oh I now understand the problem, but for that case, using the optimization option is not the correct approach. In addition as a workaround you could also exclude these deps from bring pre-bundled. Using the dev-server |
packages/angular/build/src/tools/esbuild/application-code-bundle.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kindly sign the CLA as otherwise we'll be unable to accept this change. Also please update the commit message to reflect the change and include a body with a description in the commit.
Thanks.
700c969 to
bf0b1ff
Compare
|
Thanks @alan-agius4 Code updated 👍🏻 |
packages/angular/build/src/tools/esbuild/application-code-bundle.ts
Outdated
Show resolved
Hide resolved
…packages are not inlined The current bundle logic removes any import starting with `@angular/localize` if i18n inline is active. The i18n inline transformation is only applied to internal packages. This causes an issue at run time execution of the packages which are external (pre-bundled) and therefore not inlined. The current fix aims at removing import starting with `@angular/localize` only if both i18n inline is active and there are no external packages.
bf0b1ff to
48a21b7
Compare
alan-agius4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for this.
…ternal packages (#31721) The current bundle logic removes the `@angular/localize` polyfill if i18n inline is active. However, i18n inlining is not applied on external packages (e.g. during `ng serve` for prebundled dependencies) This causes an issue at runtime execution of the packages which are external and depend on localization. With this change the `@angular/localize` polyfill is retained when external packages is truthy. (cherry picked from commit 38c2200)
…ternal packages (#31721) The current bundle logic removes the `@angular/localize` polyfill if i18n inline is active. However, i18n inlining is not applied on external packages (e.g. during `ng serve` for prebundled dependencies) This causes an issue at runtime execution of the packages which are external and depend on localization. With this change the `@angular/localize` polyfill is retained when external packages is truthy. (cherry picked from commit 38c2200)
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What is the current behavior?
@angular/localizescripts are always removed when inline transformations are enabled, even though not all the scripts/dependencies might have been transformedIssue Number: N/A
What is the new behavior?
@angular/localizescripts will only be removed when inline transformations are enabled and all the scripts/dependencies are actually transformedDoes this PR introduce a breaking change?