Skip to content
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

expose ngc i18n options #3098

Merged
merged 8 commits into from Dec 14, 2016
Merged

expose ngc i18n options #3098

merged 8 commits into from Dec 14, 2016

Conversation

tdesmet
Copy link
Contributor

@tdesmet tdesmet commented Nov 10, 2016

This allows you to pass the i18n file, format and locale to ngc from the cli command line

@googlebot
Copy link

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

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@tdesmet
Copy link
Contributor Author

tdesmet commented Nov 10, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@hansl
Copy link
Contributor

hansl commented Nov 10, 2016

@vicb could you confirm (we can talk about it tomorrow) that this change will work with the next iterations of i18n?

</file>
</xliff>`))
.then(() => appendToFile('src/app/app.component.html',
'<h1 i18n="User welcome|An introduction header for this sample">Hello i18n!</h1>'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove User welcome| (because meaning should seldom be used)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I did not know that.

Also couldn't find anything about using it seldom on the angular cookbook.

@vicb
Copy link

vicb commented Nov 11, 2016

yep let's talk tomorrow.

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff! Just one comment to add another test and it should be good!

'<h1 i18n="An introduction header for this sample">Hello i18n!</h1>'))
.then(() => ng('build', '--aot', '--i18n-file', 'src/locale/messages.fr.xlf', '--i18n-format',
'xlf', '--locale', 'fr'))
.then(() => expectFileToMatch('dist/main.bundle.js', /Bonjour i18n!/));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a negative test? Running ng build on the same project without --locale should not contain Bonjour i18n.

@tdesmet
Copy link
Contributor Author

tdesmet commented Nov 15, 2016

@hansl is this ok now?

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks great. LGTM

@hansl
Copy link
Contributor

hansl commented Nov 18, 2016

I'll rebase and merge this soon.

@shaneog
Copy link

shaneog commented Nov 23, 2016

Just a little nudge.
I have an application in production using CLI, AoT and i18n and at the moment I have to run a hacky gulp task to make it work. It would be great to get this merged.

Thanks!

@shaneog
Copy link

shaneog commented Dec 2, 2016

@hansl Is there anything we can do to get this PR into the next release?

@hansl
Copy link
Contributor

hansl commented Dec 2, 2016

Sorry, not beta 22, but beta 23. I'll rebase this today.

@rolandoldengarm
Copy link

rolandoldengarm commented Dec 6, 2016

Not sure if there's something wrong with the PR, but I've cloned the repo, linked it with "npm link", but when I run it, it is looking for a folder in someone's user directory @tdesmet @hansl ?

$ ng serve --aot --locale zh --i18nFormat xtb --i18nFile ./src/i18n/messages.zh.xtb
** NG Live Development Server is running on http://localhost:4200. **
  0% compilingpath must be a string or Buffer
TypeError: path must be a string or Buffer
    at TypeError (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.fs.readFileSync (fs.js:508:33)
    at Function.CodeGenerator.create (C:\Users\iminar\Dev\angular\modules\@angular\compiler-cli\src\codegen.ts:102:29)
    at AotPlugin._make (C:\Projects\npm\angular-cli\packages\webpack\src\plugin.ts:207:54)

I have no clue why this is happening?

I've also rebased the solution the latest Angular-CLI version which has updated dependencies to Angular Compiler, but same error...:

$ ng serve --aot --locale zh --i18nFormat xtb --i18nFile ./src/i18n/messages.zh.xtb
** NG Live Development Server is running on http://localhost:4200. **
path must be a string or Buffer
TypeError: path must be a string or Buffer
    at TypeError (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.fs.readFileSync (fs.js:508:33)
    at Function.CodeGenerator.create (C:\Projects\npm\angular-cli\node_modules\@angular\compiler-cli\src\codegen.js:83:33)
    at AotPlugin._make (C:\Projects\npm\angular-cli\packages\@ngtools\webpack\src\plugin.ts:229:60)
    at Compiler.<anonymous> (C:\Projects\npm\angular-cli\packages\@ngtools\webpack\src\plugin.ts:190:75)
    at Compiler.applyPluginsParallel (C:\Projects\npm\angular-cli\node_modules\tapable\lib\Tapable.js:193:14)
    at Compiler.<anonymous> (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:463:8)
    at Compiler.applyPluginsAsyncSeries (C:\Projects\npm\angular-cli\node_modules\tapable\lib\Tapable.js:95:46)
    at Compiler.compile (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:456:7)
    at Watching.<anonymous> (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:46:17)
    at next (C:\Projects\npm\angular-cli\node_modules\tapable\lib\Tapable.js:102:11)
    at Compiler.invalidAsyncPlugin (C:\Projects\npm\angular-cli\node_modules\webpack-dev-middleware\middleware.js:119:3)
    at next (C:\Projects\npm\angular-cli\node_modules\tapable\lib\Tapable.js:104:14)
    at Compiler.<anonymous> (C:\Projects\npm\angular-cli\node_modules\webpack\lib\CachePlugin.js:31:4)
    at Compiler.applyPluginsAsyncSeries (C:\Projects\npm\angular-cli\node_modules\tapable\lib\Tapable.js:106:13)
    at Watching._go (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:44:16)
    at Watching.<anonymous> (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:36:8)
    at Compiler.readRecords (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:367:10)
    at new Watching (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:33:16)
    at Compiler.watch (C:\Projects\npm\angular-cli\node_modules\webpack\lib\Compiler.js:204:17)
    at module.exports (C:\Projects\npm\angular-cli\node_modules\webpack-dev-middleware\middleware.js:146:27)
    at new Server (C:\Projects\npm\angular-cli\node_modules\webpack-dev-server\lib\Server.js:43:20)
    at Class.run (C:\Projects\npm\angular-cli\packages\angular-cli\tasks\serve-webpack.ts:85:22)
    at C:\Projects\npm\angular-cli\packages\angular-cli\commands\serve.ts:105:26
    at process._tickCallback (internal/process/next_tick.js:103:7)

@tdesmet
Copy link
Contributor Author

tdesmet commented Dec 7, 2016

Have you tried putting the path between double quotes?

ng serve --aot --locale zh --i18nFormat xtb --i18nFile "./src/i18n/messages.zh.xtb"

@rolandoldengarm
Copy link

Nope, same problem :( @tdesmet

$ ng serve --aot --locale zh --i18nFormat xtb --i18nFile "./src/i18n/messages.zh.xtb"
** NG Live Development Server is running on http://localhost:4200. **
  0% compilingpath must be a string or Buffer
TypeError: path must be a string or Buffer
    at TypeError (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.fs.readFileSync (fs.js:508:33)
    at Function.CodeGenerator.create (C:\Users\iminar\Dev\angular\modules\@angular\compiler-cli\src\codegen.ts:102:29)

@filipesilva
Copy link
Contributor

@rolandoldengarm that's probably a typescript problem, yesterday a few of those were cropping up. Make sure you are using 2.0.x and NOT 2.1.x.

@filipesilva
Copy link
Contributor

@tdesmet some recent changes introduced a lot of conflicts with this PR, can you rebase and fix them?

@tdesmet
Copy link
Contributor Author

tdesmet commented Dec 9, 2016

Done

@rolandoldengarm
Copy link

@filipesilva I've tried on a clean machine, with TS 2.0.10, and still the same error.
I've cloned repo, used "npm link" to link it, then "npm link angular-cli" in my project.

@rolandoldengarm
Copy link

sorry for bothering you again, but when will this PR be merged and released? We are blocked by this; I don't know how to implement i18n + AOT without this PR being merged.

@tdesmet
Copy link
Contributor Author

tdesmet commented Dec 14, 2016

@hansl @filipesilva can you guys merge this please?

supressSizes: boolean;
baseHref?: string;
aot?: boolean;
<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Git merge issue?
There are many more below as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, I'm sorry these .orig files should never have been committed.

These are artifacts from merging and shouldn't have been added I'm used to them being ignored.

Anyway I Removed them and everything should be good.

@ghidoz
Copy link

ghidoz commented Dec 14, 2016

Hi guys, thanks for the good work! Do you know when it will be merged and released? Are there any chances for the next week? Thanks!

@hansl
Copy link
Contributor

hansl commented Dec 14, 2016

Alright. Merging this. I am about to change the plugin for the next release and this LGTM, so expect it to come in the next release (which will also be ng 2.3 compatible).

@hansl
Copy link
Contributor

hansl commented Dec 14, 2016

@tdesmet could we also have an angular-cli global option for these? would that be useful?

@hansl hansl merged commit 2a0a42d into angular:master Dec 14, 2016
@tdesmet
Copy link
Contributor Author

tdesmet commented Dec 14, 2016

@hansl what do you mean?

@hansl
Copy link
Contributor

hansl commented Dec 14, 2016

Would it make sense to have i18file and i18format (and potentially locale as well?) in the angular-cli.json configuration for your app rather than having both on the command line?

Basically are those flags likely to change between two calls of ng build? I would think locale does, but i18file and format would not?

@tdesmet
Copy link
Contributor Author

tdesmet commented Dec 14, 2016

Format stays the same but locale and file do not in this implementation. File is translated messages for the locale. So if locale is fr then file is messages.fr.xlf anf if locale is de then file is messages.de.xlf.

I'm thinking we could probably make this easier by determining the file name by the given locale. Bu this would be less flexible as all users need to follow the same naming pattern.

Currently in my app I have a locales directory and each locale is a subdirectory containing a messages.locale.xlf file. My build process scans the locales directory and executes the build command for each found subdirectory.
By typing this I realise I could just name all xlf files the same since they all live in seperate directories.

For me it would be great if we could implement something similar in the cli. But not sure if this workflow would work for others...

@hansl
Copy link
Contributor

hansl commented Dec 14, 2016 via email

MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants