feat(ivy): support i18n without closure by ocombe · Pull Request #28689 · angular/angular

ocombe · 2019-02-13T13:52:28Z

So far using runtime i18n with ivy meant that you needed to use Closure and goog.getMsg (or a polyfill). This PR changes the compiler to only output closure code when the compiler option i18nUseClosure is set to true. This option is set to false by the compiler-cli when the option i18nInFile is used, which is what happens when you load a translations file.
This means that if you use the Angular CLI with ivy and load a translations file, you can use i18n and the application will not throw at runtime.
For now it will not translate your application, but at least you can try ivy without having to remove all of your i18n code and configuration.

petebacondarwin

LGTM for ngcc

AndrewKushnir

@ocombe looks good! Just left a couple minor comments. Thank you.

ocombe · 2019-02-18T15:06:58Z

Blocked by https://angular-team.atlassian.net/browse/FW-1076 (bazel issue)

gregmagolan · 2019-02-26T09:34:58Z

Blocked by https://angular-team.atlassian.net/browse/FW-1076 (bazel issue)

Alex got #21883 merged which should resolve FW-1076. You should be unblocked on master.

ocombe · 2019-02-26T12:58:30Z

@gregmagolan just rebased on master and it doesn't change anything for me. It still doesn't take the tsconfig into consideration. Am I supposed to change something else to the bazel build file to make this work?
edit: it works now, thanks

IgorMinar

can you please address the comments I posted and split this PR into two - one for cli update to beta.11 and one for the remaining changes. that way we can isolate the payload size regression and attribute it to this change or an issue in cli. thanks

IgorMinar · 2019-04-05T21:13:10Z

can you please not change this. if you need a newer version, then you should update the root package.json and not this one.

we talked about this at the framework meeting yesterday and it was decided to only change this in the ivy integration apps for now

can you please share why?

my concern is that we are making our integration testing infrastructure more fragile if we make this change and we are increasing technical debt. I'd prefer not to make a change like that unless we really really have to (and then there should be a follow up work to remove this temporary hack which should be tracked and scheduled so that we don't forget).

IgorMinar · 2019-04-05T21:27:00Z

why will this happen? if this really is a TODO then we should likely have an item in jira for this. If this is just a note then make it a note and not a TODO

Because ɵi18nConfigureLocalize is just temporary (as explained in its api docs) until we can work on the service. We needed something for our tests to make i18n work without closure so that people can test ivy even if they use i18n.
There is a already jira ticket for the runtime service (not specifically this todo, but it's part of it).

let me try to explain my question better: if I'm reading this todo in the codebase a month from now, what should I do, how should I interpret it, or how can I learn more about what this comment stands for?

In its current form, all I can do is ask you personally what this is about and where I can learn more, and that doesn't scale.

I've updated the todo, let me know if it is still unclear.

ocombe · 2019-04-05T22:34:25Z

@IgorMinar the size regression was present before the cli change. I tried to update the cli version to see if my change to tree shake ngI18nClosureMode could fix the regression (angular/angular-cli#13905) but it didn't change anything (in fact it added 2 bytes).

ocombe · 2019-04-08T09:06:36Z

I removed the cli changes, and rebased onto master and it fixed the size regression issue. @IgorMinar can you approve the PR now? :)

IgorMinar

lgtm except for that one todo

kara · 2019-04-09T19:14:01Z

presubmit

So far using runtime i18n with ivy meant that you needed to use Closure and `goog.getMsg` (or a polyfill). This PR changes the compiler to output both closure & non-closure code, while the unused option will be tree-shaken by minifiers. This means that if you use the Angular CLI with ivy and load a translations file, you can use i18n and the application will not throw at runtime. For now it will not translate your application, but at least you can try ivy without having to remove all of your i18n code and configuration.

IgorMinar · 2019-04-11T15:03:01Z

previews presubmit failed, so here is a new one after the last set of changes: http://test/OCL:243069482:BASE:243069609:1554994952547:fae7a7a0

angular-automatic-lock-bot · 2019-09-14T23:21:42Z

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.}

ocombe requested a review from IgorMinar as a code owner February 13, 2019 13:52

ocombe requested review from a team February 13, 2019 13:52

googlebot added the cla: yes label Feb 13, 2019

petebacondarwin approved these changes Feb 13, 2019

View reviewed changes

ocombe added the state: WIP label Feb 13, 2019

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch from 7742649 to 5019827 Compare February 13, 2019 14:07

ocombe added target: major This PR is targeted for the next major release area: i18n Issues related to localization and internationalization comp: ivy labels Feb 13, 2019

ngbot Bot added this to the needsTriage milestone Feb 13, 2019

AndrewKushnir mentioned this pull request Feb 13, 2019

ivy: i18n messages identifiers have dots in their name #28080

Closed

mhevery approved these changes Feb 13, 2019

View reviewed changes

mhevery requested a review from AndrewKushnir February 13, 2019 19:34

mhevery self-assigned this Feb 13, 2019

AndrewKushnir approved these changes Feb 13, 2019

View reviewed changes

Comment thread packages/compiler-cli/src/ngtsc/program.ts Resolved

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

ocombe added the state: blocked label Feb 18, 2019

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch from e2c6635 to 8ef7ee5 Compare February 20, 2019 09:26

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch from 8ef7ee5 to a578389 Compare February 26, 2019 12:57

gregmagolan mentioned this pull request Feb 26, 2019

build(bazel): allow a user to control a subset of angularCompilerOption in their tsconfig file when using ng_module #28995

Closed

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch 2 times, most recently from f4f2e48 to aebd03a Compare February 27, 2019 21:01

ocombe added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP state: blocked labels Feb 27, 2019

AndrewKushnir removed the state: blocked label Apr 4, 2019

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch 4 times, most recently from 7618e6a to 3afbb98 Compare April 5, 2019 16:20

mhevery approved these changes Apr 5, 2019

View reviewed changes

ocombe added the action: merge The PR is ready for merge by the caretaker label Apr 5, 2019

IgorMinar suggested changes Apr 5, 2019

View reviewed changes

ocombe removed the action: merge The PR is ready for merge by the caretaker label Apr 8, 2019

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch 2 times, most recently from 32d8695 to 29445e6 Compare April 8, 2019 08:46

ocombe added the action: merge The PR is ready for merge by the caretaker label Apr 8, 2019

ocombe requested a review from IgorMinar April 8, 2019 09:05

ocombe requested review from IgorMinar and alxhub and removed request for IgorMinar April 8, 2019 13:29

alxhub approved these changes Apr 8, 2019

View reviewed changes

IgorMinar approved these changes Apr 9, 2019

View reviewed changes

IgorMinar 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 Apr 9, 2019

ocombe removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 9, 2019

ocombe force-pushed the feat/ivy/runtime-i18n-shim branch 2 times, most recently from 129e4c7 to 092e71b Compare April 11, 2019 09:18

IgorMinar approved these changes Apr 11, 2019

View reviewed changes

filipesilva mentioned this pull request Apr 16, 2019

fix(core): use shakeable global definitions #29929

Closed

3 of 14 tasks

Conversation

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

Uh oh!

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ocombe commented Feb 18, 2019

Uh oh!

gregmagolan commented Feb 26, 2019

Uh oh!

ocombe commented Feb 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

IgorMinar Apr 5, 2019

Choose a reason for hiding this comment

Uh oh!

ocombe Apr 5, 2019

Choose a reason for hiding this comment

Uh oh!

IgorMinar Apr 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IgorMinar Apr 5, 2019

Choose a reason for hiding this comment

Uh oh!

ocombe Apr 5, 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!

IgorMinar Apr 9, 2019

Choose a reason for hiding this comment

Uh oh!

ocombe Apr 9, 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!

ocombe commented Apr 5, 2019

Uh oh!

ocombe commented Apr 8, 2019

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

kara commented Apr 9, 2019

Uh oh!

IgorMinar commented Apr 11, 2019

Uh oh!

angular-automatic-lock-bot Bot commented Sep 14, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ocombe commented Feb 13, 2019 •

edited

Loading

ocombe commented Feb 26, 2019 •

edited

Loading

ocombe Apr 5, 2019 •

edited

Loading

ocombe Apr 9, 2019 •

edited

Loading