feature(i18n): implement xi18n command #3340

cladera · 2016-12-01T11:36:12Z

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

cladera · 2016-12-01T11:39:42Z

packages/angular-cli/tasks/extract-i18n.ts

+        get: (s: string) => {
+          if (!host.fileExists(s)) {
+            // Return empty string to avoid extractor stop processing
+            return Promise.resolve('');


This should solve the problem with SASS partials.

However, it will affect other cases so I wonder if is it the best solution.

I'd like to start a discussion here: Should the extractor stop if a resource is not found? Does not make more sense to keep processing files and perhaps showing a warning instead of throwing an error?

Would it be useful to compile less/sass files? Also, please note that in the future we will support other format, even for templates (pug comes to mind).

Are you available on gitter right now? If so, contact me and we can review this interactively for fast tracking.

I don't see the point of compiling less/sass files since they will never contain i18n messages, actually I rather ignore them. Templates, though, make a lot more sense.

cladera · 2016-12-01T11:47:36Z

tests/acceptance/extract-i18n.spec.js

+    return tmp.setup('./tmp').then(function() {
+      process.chdir('./tmp');
+    }).then(function() {
+      return ng(['new', 'foo', '--link-cli']);


ng new should be executed with --skip-npm and --skip-bower flags. But if we do so, the extractor won't work. I guess it is because the extractor seems to compile the project. If the dependencies are not installed the command throws an exception because eventually a module will not be found.

If the beforeEach has to install the dependencies it takes too much time. That's why I skipped the test because I though installing all vendors is unacceptable.

Any idea how to make this work?

Bower should never install now, since we removed that code and those dependencies from Angular CLI.

I think this test would be better suited as an e2e test since you change files and do a lot of FS work. If you need help adding an e2e test, reach me out on gitter and/or by email. I'll help you.

hansl

I'm going to need some time to read this since I'm lacking a lot of information regarding i18n. At first look this looks good (outside of maybe minor nits).

Give me a few days for a full review please.

hansl · 2016-12-01T19:33:47Z

tests/acceptance/extract-i18n.spec.js

+    return tmp.setup('./tmp').then(function() {
+      process.chdir('./tmp');
+    }).then(function() {
+      return ng(['new', 'foo', '--link-cli']);


Bower should never install now, since we removed that code and those dependencies from Angular CLI.

I think this test would be better suited as an e2e test since you change files and do a lot of FS work. If you need help adding an e2e test, reach me out on gitter and/or by email. I'll help you.

cladera · 2016-12-02T09:40:01Z

Replaced acceptance tests by e2e tests. Let me know if they satisfy your requirements.

hansl · 2016-12-06T18:44:14Z

Could you rebase on the latest master? I think those e2e tests are failing because of an old error.

cladera · 2016-12-06T18:59:47Z

Done!

hansl · 2016-12-07T00:43:18Z

packages/angular-cli/tasks/extract-i18n.ts

+          case 'xliff':
+          case 'xlf':
+          default:
+            const htmlParser = new compiler.I18NHtmlParser(new compiler.HtmlParser());


If the format is not one we understand, we should just error out. That way the user will know there's a typo.

cladera · 2016-12-07T11:12:20Z

Pushed a new version that throws an error if format argument is none of allowed.

However, since 2.3.0-rc.0 compiler's Extractor no longer accepts a resourceLoader function as a parameter which changes completely this implementation. I'll have to take a look to new i18n extracting API.

hansl · 2016-12-07T22:21:49Z

@vicb Did we change the interface of the Extractor?

vicb · 2016-12-08T00:17:45Z

The Serializer interface has changed in 2.3

cladera · 2016-12-13T10:52:25Z

@vicb How new Serializer's interface affects to resource loading?

Hawkzfan · 2017-01-02T13:09:15Z

Hi @cladera
Hi @vicb

what is the current state of this pull request? How would you have to rewrite this task...
Thank you so much!

hansl · 2017-01-06T23:32:23Z

@cladera Many changes came in 2.4.0. This PR basically needs to be redone, but it's not that big of a deal. We added a private API in Angular that can be used to extract stuff, without worrying about changes to the Extractor class.

We probably wants to run through a full webpack build while doing the extraction so that we have a compilation and can build sass/less/pug files. Do you know how to do that? I can help you finish this PR.

hansl · 2017-01-30T21:20:58Z

tests/e2e/tests/i18n/extract-default.ts

+export default function() {
+  const testComponentDir = join('src/app', 'i18n-test');
+  return ng('generate', 'component', 'i18n-test')
+    .then(() => deleteFile(join(testComponentDir, 'i18n-test.component.html')))


You don't need to delete it, since you're overwriting it. Also, you don't need to keep a variable for the path.

hansl · 2017-01-30T21:21:07Z

tests/e2e/tests/i18n/extract-output.ts

+export default function() {
+  const testComponentDir = join('src/app', 'i18n-test');
+  return ng('generate', 'component', 'i18n-test')
+    .then(() => deleteFile(join(testComponentDir, 'i18n-test.component.html')))


hansl · 2017-01-30T21:21:16Z

tests/e2e/tests/i18n/extract-xmb.ts

+export default function() {
+  const testComponentDir = join('src/app', 'i18n-test');
+  return ng('generate', 'component', 'i18n-test')
+    .then(() => deleteFile(join(testComponentDir, 'i18n-test.component.html')))


hansl · 2017-01-30T21:30:01Z

packages/@ngtools/webpack/src/extract_i18n_plugin.ts

+  private _make(compilation: any, cb: (err?: any, request?: any) => void) {
+    this._compilation = compilation;
+    if (this._compilation._ngToolsWebpackXi18nPluginInstance) {
+      return cb(new Error('An @ngtools/webpack xi18n plugin already exist for ' +


You should also validate that we don't have a _ngToolsWebpackPluginInstance, since that would seem to be an error.

Changes pushed. Travis build failed though. I don't understand why it failed, can you take a look and let me know if I broke something.

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

Revisit e2e tests.

hansl · 2017-02-08T03:18:57Z

Thank you so much, @cladera. This is great!

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

JohannesHoppe · 2017-02-15T17:52:27Z

ng i18n is avalailable in @angular/cli: 1.0.0-beta.31
but i don't see any entry for this in the changelog?

anyway, thanks! very convenient! 👍

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

angular-automatic-lock-bot · 2019-09-11T20:42:25Z

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 Dec 1, 2016

cladera commented Dec 1, 2016

View reviewed changes

cladera mentioned this pull request Dec 1, 2016

[Discussion] angular-cli and i18n (ng-xi18n + ngc) #2201

Closed

hansl suggested changes Dec 1, 2016

View reviewed changes

cladera force-pushed the extract-i18n-command branch from 2996b2b to 279d32a Compare December 2, 2016 09:38

cladera force-pushed the extract-i18n-command branch from 279d32a to 1347999 Compare December 6, 2016 18:57

hansl reviewed Dec 7, 2016

View reviewed changes

cladera force-pushed the extract-i18n-command branch from 1347999 to 0e75cf6 Compare December 7, 2016 11:06

filipesilva added the pr_action: review label Dec 8, 2016

filipesilva assigned hansl Dec 8, 2016

cladera force-pushed the extract-i18n-command branch 5 times, most recently from 9b8d75f to 70f1372 Compare January 27, 2017 09:13

hansl removed the pr_action: review label Jan 27, 2017

hansl suggested changes Jan 30, 2017

View reviewed changes

cladera force-pushed the extract-i18n-command branch from 70f1372 to 5a0fbd6 Compare January 31, 2017 08:44

cladera added 2 commits February 7, 2017 09:24

feature(i18n): implement i18n command

1eb2a87

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

Remove acceptance tests and create e2e tests

878282e

cladera added 6 commits February 7, 2017 09:24

Reject task if format argument is unknown

0bd69d1

Feature redone to use ngtools api

a28da7d

Fix code after rebase master

f5de5be

Fix e2e tests after rebase master

ccc7aed

Assert an AotPlugin instance is present in webpack compilation.

3195183

Revisit e2e tests.

Rebase master

18adc30

cladera force-pushed the extract-i18n-command branch from 5a0fbd6 to 18adc30 Compare February 7, 2017 09:18

hansl approved these changes Feb 8, 2017

View reviewed changes

hansl merged commit 8602536 into angular:master Feb 8, 2017

hansl mentioned this pull request Feb 9, 2017

Tracking: support extraction of i18n files #3920

Closed

filipesilva mentioned this pull request Feb 14, 2017

docs: add i18n story #4480

Closed

asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017

feature(i18n): implement xi18n command (angular#3340)

22bd516

Implement i18n messages extractor. Contrary to @angular/complier-cli's command it will not throw an error if a resource is not found.

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(i18n): implement xi18n command #3340

feature(i18n): implement xi18n command #3340

cladera commented Dec 1, 2016

cladera Dec 1, 2016 •

edited

Loading

hansl Dec 7, 2016

cladera Dec 7, 2016

cladera Dec 1, 2016

hansl Dec 1, 2016

hansl left a comment

hansl Dec 1, 2016

cladera commented Dec 2, 2016

hansl commented Dec 6, 2016

cladera commented Dec 6, 2016

hansl Dec 7, 2016

cladera commented Dec 7, 2016

hansl commented Dec 7, 2016

vicb commented Dec 8, 2016

cladera commented Dec 13, 2016

Hawkzfan commented Jan 2, 2017 •

edited

Loading

hansl commented Jan 6, 2017

hansl Jan 30, 2017

hansl Jan 30, 2017

hansl Jan 30, 2017

hansl Jan 30, 2017

cladera Jan 31, 2017

hansl commented Feb 8, 2017

JohannesHoppe commented Feb 15, 2017

angular-automatic-lock-bot bot commented Sep 11, 2019

feature(i18n): implement xi18n command #3340

feature(i18n): implement xi18n command #3340

Conversation

cladera commented Dec 1, 2016

cladera Dec 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cladera commented Dec 2, 2016

hansl commented Dec 6, 2016

cladera commented Dec 6, 2016

Choose a reason for hiding this comment

cladera commented Dec 7, 2016

hansl commented Dec 7, 2016

vicb commented Dec 8, 2016

cladera commented Dec 13, 2016

Hawkzfan commented Jan 2, 2017 • edited Loading

hansl commented Jan 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansl commented Feb 8, 2017

JohannesHoppe commented Feb 15, 2017

angular-automatic-lock-bot bot commented Sep 11, 2019

cladera Dec 1, 2016 •

edited

Loading

Hawkzfan commented Jan 2, 2017 •

edited

Loading