feat(i18n): angular i18n native implementation #1442

ruffiem · 2016-10-08T00:08:45Z

angular i18n native implementation for JiT processing
~~added systemjs-text-plugin to allow systemjs deal with XLIFF files~~
application entry point at app.through.ts in seed/tasks folder
npm run i18n task to extract content and create former messages.xlf
translation files to be stored in src/client/assets/locale

…mentation

mgechev · 2016-10-08T00:14:31Z

@ruffiem I took only a brief look at your changes but it looks great so far! Thanks for the PR, I'll merge the it tonight or tomorrow morning.

ruffiem · 2016-10-08T00:20:12Z

Thanks! Just saw there's something wrong with the production build, it completes but breaks at runtime on "System.import is not a function" in i18n.providers.ts. 😠

ruffiem · 2016-10-08T10:11:18Z

Working on the issue, hope to fix it by the end of the day. Does systemjs text plugin may cause such error ?

mgechev · 2016-10-08T20:28:01Z

Yes, in production we are excluding SystemJS from the bundle. I'd recommend you to use the Angular's http service instead. Otherwise it looks good to me!

ruffiem · 2016-10-08T20:41:28Z

Great, thanks. I didn't know System.import was just an xhr :) I will refactor a little, run a few tests and push when everything seems correct.

mgechev · 2016-10-08T21:02:31Z

@ruffiem another small thing - we also support AoT compilation so you'd have to update main-prod.ts because it has different bootstrap logic.

ruffiem · 2016-10-09T01:05:07Z

Ok now it does work on both dev and prod but we have a bigger challenge : when scanning the code for i18n attributes, angular gives a specific id based on the textnode format (white spaces, returns, breaking spaces...). Thing is, with minification/uglification we lose the format, the id is changing and the translation is lost. Can't think of anything "easy" to fix it right now.

EDIT: Or maybe we can minify the XLF as well to have the same formatting ?

mgechev · 2016-10-09T01:39:58Z

The id is changed by uglify performing mangling? If so, we can disable mangling for build.prod and leave it only for build.prod.exp where we may not have such problem because of the AoT.

ruffiem · 2016-10-09T10:24:54Z

Exactly.

Also, mangling the XLF is not possible since ids are hardcoded in the file.

I created an issue on the angular repo as they should rely on content instead of formatting IMO.

…s, bootstrap fix

ruffiem · 2016-10-09T19:20:48Z

I have updated the text content to avoid translation to break from the seed on prod, ids won't change but the text has to be written inline.

<h1 i18n>Howdy!</h1>

not

<h1 i18n>
     Howdy
!
</h1>

Also, TP from main.ts were causing ng-xi18n fn to break. It's fixed now.

mgechev · 2016-10-09T19:22:07Z

@ruffiem you can also disable mangling, which should be fine.

ruffiem · 2016-10-09T19:24:37Z

I have tried to disable mangling but the result was the same (except that the ids were changing but still not matching the ones from the XLF). Did you try too ?

mgechev · 2016-10-09T19:25:26Z

@ruffiem I'll give it a try later today. I'm releasing beta.1 of codelyzer today with bug fixes.

mgechev · 2016-10-09T21:39:05Z

I took a final look. It looks great, thanks for the effort one more time. My suggestion will be to keep it disabled by default because a lot of people will be confused by the bootstrap process.

Lets keep main.ts like this:

/**
 * Bootstraps the application and makes the ROUTER_PROVIDERS and the APP_BASE_HREF available to it.
 * @see https://angular.io/docs/ts/latest/api/platform-browser-dynamic/index/bootstrap-function.html
 */
import { enableProdMode } from '@angular/core';
// The browser platform with a compiler
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
// Load i18n providers
// import { TranslationProviders } from './i18n.providers';

// The app module
import { AppModule } from './app.module';

if (String('<%= ENV %>') === 'prod') { enableProdMode(); }

// Compile and launch the module with i18n providers
// let TP = new TranslationProviders();
// TP.getTranslationFile().then((providers: any) => {
//   const options: any = { providers };
platformBrowserDynamic().bootstrapModule(AppModule/*, options*/);
// });

I've also removed the comments for the Service Workers that I added in the past because they are out of date. We can add a wiki article of how the i18n can be enabled (basically uncomment the TranslationProviders and the xlf file generation).

Finally commenting in the language switching logic. Currently directly accessing window and localStorage will prevent the app from running in Web Workers, so it might be better to not have this in master.

What do you think about this?

ruffiem · 2016-10-09T22:28:15Z

Sounds great, I will make a wiki page about how to use i18n if you allow me to. And thanks to you for the seed, do not hesitate to tell me if you need help or something 👍

mgechev · 2016-10-09T22:35:16Z

Sounds good! Looking for the updates in the PR and the wiki page, then I'll be ready to merge.

ruffiem · 2016-10-10T07:16:31Z

Ok, should be fine now, I've also updated the package.json to match current master.

ruffiem · 2016-10-10T10:08:33Z

Since i18n is an option, maybe we should create a few options in ~~project.config.ts~~ seed.config.ts. The language directory is hardcoded in the AoT task which is not a great option 👎 what do you think ?

Bigous

Great Job @ruffiem . Just a minor change and a wish :D

Bigous · 2016-10-10T13:55:28Z

src/client/app/i18n.providers.ts

+      );
+      xhr.onerror = () => reject(noProviders);
+      xhr.send();
+    });


I think we could use the Angular2 HTTP provider here instead of XMLHttpRequest.

I tried but it was resolving to undefined maybe I was missing something...

@Bigous this service is invoked before bootstrap so I'm not sure if it is possible since the Angular's i18n functionality is part of the compilation process (in this case JiT).

@mgechev True. It should be a service where the Application module depends so... or keep it as it is :)

Bigous · 2016-10-10T13:57:07Z

src/client/app/home/home.component.html

-<p>
-  Howdy! Here's a list of awesome computer scientists. Do you know any others? Add to the list yourself.
-</p>
+<p>Howdy! Here's a list of awesome computer scientists. Do you know any others? Add to the list yourself.</p>


Could you add some more examples on use (like how do we can translate numbers and variables and/or plurals)

Yes, totally, but it seems like pluralization is not working well (especially 'other'). plnkr

mgechev · 2016-10-10T17:17:34Z

@ruffiem good wiki page!

mgechev · 2016-10-10T17:25:22Z

tools/tasks/seed/app.through.ts

@@ -0,0 +1 @@
+import '../../../src/client/app/main';


@ruffiem this file doesn't seem used anywhere, right?

@mgechev it's used by ng-xi18n as an entry point to the application. Since ng-xi18n relies on the tsconfig.json and src folder it excluded, I needed to find a way in 🙄

I got it. Can you pass src/client/tsconfig.json as argument of ng-xi18n? For instance in ngc it'll be: ngc -p src/client/tsconfig.json.

There's no -p option with ng-xi18n yet but it's being discussed on the angular repo (2201).

Thanks! I added this as last step in order to not pollute the seed's tasks directory.

mru added 4 commits October 8, 2016 00:48

(enhancement) i18n: added angular native i18n support - partial imple…

f043eb5

…mentation

(enhancement) i18n: added translation file

af771a5

Merge branch 'master' of https://github.com/mgechev/angular-seed

489cd81

(enhancement) i18n: minor fix

747c484

refactored i18n.providers

a873aeb

mru added 3 commits October 9, 2016 03:08

removed systemjs-text-plugin from deps

40f6f91

updated seed.config.ts (removed systemjs text plugin)

8822788

(fix) e2e: modified app.component end-to-end to match new navbar

4023323

updated text content to avoid translations break, updated dependencie…

cd4df83

…s, bootstrap fix

mgechev approved these changes Oct 9, 2016

View reviewed changes

mgechev changed the title ~~(enhancement) i18n: angular i18n native implementation~~ feat(i18n): angular i18n native implementation Oct 9, 2016

mru added 2 commits October 10, 2016 08:56

updated i18n disable by default

9b1a020

downgraded dependencies in package.json

5b2e29d

mru added 3 commits October 10, 2016 09:21

merged from seed

745ee4a

fixed conflict in package.json

0e54cc9

updated for AoT

37ac59d

Bigous reviewed Oct 10, 2016

View reviewed changes

mgechev merged commit 4c8ad43 into mgechev:master Oct 10, 2016

mgechev reviewed Oct 10, 2016

View reviewed changes

feat(i18n): angular i18n native implementation #1442

feat(i18n): angular i18n native implementation #1442

Uh oh!

Conversation

ruffiem commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgechev commented Oct 8, 2016

Uh oh!

ruffiem commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruffiem commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgechev commented Oct 8, 2016

Uh oh!

ruffiem commented Oct 8, 2016

Uh oh!

mgechev commented Oct 8, 2016

Uh oh!

ruffiem commented Oct 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgechev commented Oct 9, 2016

Uh oh!

ruffiem commented Oct 9, 2016

Uh oh!

ruffiem commented Oct 9, 2016

Uh oh!

mgechev commented Oct 9, 2016

Uh oh!

ruffiem commented Oct 9, 2016

Uh oh!

mgechev commented Oct 9, 2016

Uh oh!

mgechev commented Oct 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruffiem commented Oct 9, 2016

Uh oh!

mgechev commented Oct 9, 2016

Uh oh!

ruffiem commented Oct 10, 2016

Uh oh!

ruffiem commented Oct 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bigous left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgechev Oct 10, 2016 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgechev commented Oct 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ruffiem commented Oct 8, 2016 •

edited

Loading

ruffiem commented Oct 8, 2016 •

edited

Loading

ruffiem commented Oct 8, 2016 •

edited

Loading

ruffiem commented Oct 9, 2016 •

edited

Loading

mgechev commented Oct 9, 2016 •

edited

Loading

ruffiem commented Oct 10, 2016 •

edited

Loading

mgechev Oct 10, 2016 •

edited

Loading