feat(i18n): initial `$localize` functionality by petebacondarwin · Pull Request #31609 · angular/angular

petebacondarwin · 2019-07-17T15:10:19Z

~~Blocked on #31943~~

TODOs

For this PR:

fix AOT tests to provide a runtime implementation of $localize
fix material legacy tests to update with @angular/localize etc (perhaps via a migration?)
fix integration tests to update with @angular/localize etc (perhaps via a migration?)

For next PR?

write the update migration schematic
write the new project schematic
write compile-time inliner (Babel plugin/standalone tool)

googlebot · 2019-07-17T16:42:57Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only "I consent." in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-07-18T09:34:50Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot · 2019-07-18T14:48:39Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only "I consent." in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-07-18T20:24:55Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

filipesilva · 2019-08-09T12:29:28Z

We should be able to not introduce these side effects, depending on the way the check is made.

IgorMinar

haven't reviewed everything, but this is what I've got so far.

IgorMinar · 2019-08-12T16:59:35Z

right. this shouldn't be a peer dep because you'd be forcing everyone to install it or get a warning. I think we need to settle for a unspecified dependency - this is why we want the runtime and compile time warning about missing @angular/localize.

trotyl · 2019-08-13T04:39:54Z

This isn't a polyfill even when made global function, polyfills.ts is not the place to import arbitrary global libraries.

Zone.js can be considered a polyfill only because it has a stage-0 proposal.

Should user add jquery here if they want to access $ everywhere?

I agree it is not a polyfill.
The only requirement is that @angular/localize is imported before any use of the $localize tag.
So it would be perfectly reasonable to add this just before any import of @angular/core - but only if you will only use $localize in Angular templates or Angular application code. Therefore this could be added in main.ts instead.

But... @mhevery argued that since $localize is expected to be a general purpose i18n utility, it could be used in code before Angular is loaded, in which case putting it in polyfills.ts is safer, since $localzie is then guaranteed to be there before any of the application code runs.

One example of this is when using runtime inlining of the messages and loading the translations in a script that is executed in the browser before the main.ts.

A more semantically accurate approach would be to change the build pipeline to load a new file between polyfills.ts and main.ts, (e.g. localize.ts) which is responsible for importing @angular/localize and setting up translations. But we felt that this would be too large a change to accommodate for v9.

ngbot · 2019-08-30T09:04:20Z

I see that you just added the PR action: merge label, but the following checks are still failing:
     conflicts with base branch "master"
     status "ci/circleci: legacy-unit-tests-saucelabs" is failing
     missing required status "ci/circleci: publish_snapshot"
     2 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

The files in the pattern do not exist.

This commit changes the Angular compiler (ivy-only) to generate `$localize` tagged strings for component templates that use `i18n` attributes. BREAKING CHANGE Since `$localize` is a global function, it must be included in any applications that use i18n. This is achieved by importing the `@angular/localize` package into an appropriate bundle, where it will be executed before the renderer needs to call `$localize`. For CLI based projects, this is best done in the `polyfills.ts` file. ```ts import '@angular/localize'; ``` For non-CLI applications this could be added as a script to the index.html file or another suitable script file.

This has been replaced by the `$localize` tag.

This has been replaced by the `loadTranslations()` function in `@angular/localize/run_time`.

This test uses localization in the `AppComponent` component: * an `i18n` attribute in the template * a call to the `$localize` tag in the component constructor

mhevery · 2019-08-30T19:33:03Z

GLOBAL APPROVAL: misko

mhevery · 2019-09-03T16:47:34Z

This needs to be reverted as it breaks g3.

/checkbox_logo.closure.js:47: ERROR - [JSC_UNDEFINED_VARIABLE] variable $localize is undeclared
    I18N_6 = $localize `User portrait`;
             ^^^^^^^^^

angular-automatic-lock-bot · 2019-10-04T16:06:41Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jul 17, 2019

petebacondarwin added the state: WIP label Jul 17, 2019

petebacondarwin force-pushed the i18n-localize-init branch 2 times, most recently from fa4213d to 84eff89 Compare July 17, 2019 16:07

mhevery approved these changes Jul 17, 2019

View reviewed changes

Comment thread tsconfig.json Outdated

Comment thread packages/i18n/localize/src/hash.ts Outdated

Comment thread packages/i18n/localize/src/hash.ts Outdated

mhevery added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jul 17, 2019

googlebot added cla: no and removed cla: yes labels Jul 17, 2019

matsko added the area: i18n Issues related to localization and internationalization label Jul 17, 2019

ngbot Bot added this to the needsTriage milestone Jul 17, 2019

ocombe approved these changes Jul 18, 2019

View reviewed changes

petebacondarwin force-pushed the i18n-localize-init branch from dcc05e6 to 78021d4 Compare July 18, 2019 09:34

googlebot removed the cla: no label Jul 18, 2019

googlebot added cla: no and removed cla: yes labels Jul 18, 2019

petebacondarwin force-pushed the i18n-localize-init branch from 0afe5b9 to feec4a8 Compare July 18, 2019 20:24

googlebot added cla: yes and removed cla: no labels Jul 18, 2019

petebacondarwin marked this pull request as ready for review July 18, 2019 20:25

petebacondarwin requested a review from IgorMinar as a code owner July 18, 2019 20:25

petebacondarwin requested a review from a team July 18, 2019 20:25

petebacondarwin force-pushed the i18n-localize-init branch 2 times, most recently from 0b49661 to 416d8b6 Compare July 18, 2019 21:06

petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews state: WIP labels Jul 18, 2019

ocombe reviewed Aug 7, 2019

View reviewed changes

Comment thread packages/core/src/render3/i18n.md Outdated

Comment thread packages/core/src/render3/i18n.ts Outdated

Comment thread packages/core/src/render3/i18n.ts Outdated

Comment thread packages/core/src/render3/i18n.ts Outdated

Comment thread packages/core/src/render3/i18n.ts Outdated

petebacondarwin force-pushed the i18n-localize-init branch from e3ee66b to 31738e2 Compare August 8, 2019 21:41

petebacondarwin added state: blocked and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Aug 8, 2019

petebacondarwin force-pushed the i18n-localize-init branch 3 times, most recently from 16465bc to 79fe4fc Compare August 9, 2019 12:09

filipesilva reviewed Aug 9, 2019

View reviewed changes

petebacondarwin force-pushed the i18n-localize-init branch 4 times, most recently from 89c9b11 to eba97c4 Compare August 10, 2019 06:39

IgorMinar reviewed Aug 12, 2019

View reviewed changes

petebacondarwin commented Aug 12, 2019

View reviewed changes

Comment thread packages/core/testing/src/testing.ts Outdated

trotyl reviewed Aug 13, 2019

View reviewed changes

mhevery approved these changes Aug 28, 2019

View reviewed changes

Comment thread integration/cli-hello-world-ivy-compat/src/polyfills.ts Outdated

Comment thread packages/localize/runtime/src/translate.ts Outdated

Comment thread packages/localize/runtime/src/translate.ts Outdated

ocombe mentioned this pull request Aug 30, 2019

I18n localize - compile-time #32105

Closed

14 tasks

petebacondarwin added 8 commits August 30, 2019 10:11

test(common): remove redundant line from karma config

56bc176

The files in the pattern do not exist.

feat(ivy): implement $localize() global function

cb0d6f8

refactor(ivy): remove i18nLocalize instruction

d6c4e5d

This has been replaced by the `$localize` tag.

refactor(ivy): remove i18nConfigureLocalize instruction

a17b9d4

This has been replaced by the `loadTranslations()` function in `@angular/localize/run_time`.

docs(core): update the i18n design doc

7b5119c

test(ivy): add cli-hello-world-ivy-i18n integration test

a767d7d

This test uses localization in the `AppComponent` component: * an `i18n` attribute in the template * a call to the `$localize` tag in the component constructor

test: make cli-hello-world-ivy-minimal/debug-test.sh executable

3b9909f

petebacondarwin mentioned this pull request Sep 4, 2019

[IVY] i18n not working yet? #32311

Closed

Conversation

petebacondarwin commented Jul 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

googlebot commented Jul 17, 2019

Uh oh!

googlebot commented Jul 18, 2019

Uh oh!

googlebot commented Jul 18, 2019

Uh oh!

googlebot commented Jul 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

filipesilva Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IgorMinar Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trotyl Aug 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ngbot Bot commented Aug 30, 2019

Uh oh!

mhevery commented Aug 30, 2019

Uh oh!

mhevery commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angular-automatic-lock-bot Bot commented Oct 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

petebacondarwin commented Jul 17, 2019 •

edited

Loading

trotyl Aug 13, 2019 •

edited

Loading

petebacondarwin Aug 16, 2019 •

edited

Loading

mhevery commented Sep 3, 2019 •

edited

Loading