Implement TranslationService v2 (ckeditor5-dev part) #314

ma2ciek · 2017-11-15T13:57:57Z

Suggested merge commit message (convention)

Feature: Implement TranslationService v2 (ckeditor5-dev part). Closes ckeditor/ckeditor5#666. Closes ckeditor/ckeditor5#624.

BREAKING CHANGE: CKEditorWebpackPlugin plugin supports now language and additionalLanguages options instead of languages. If only language is set, the plugin will translate code directly into the main bundle. When additionalLanguages are provided, then the plugin will output bundle with the main language and rest translation files separately.

Additional information

It's the main change that's required by Implement translation service (i18n) v2 ckeditor5#624.
It allows providing internalization for multiple languages or single language.
It allows creating js translation files, that can be added to the HTML file alongside with the built editor.
Parts of the translation service should be easier to maintain and more CKEditor5 independent, so they can be available to use outside, e.g. from the Letters codebase.
Few missing tests added.

…and translation services.

…ce` files.

ma2ciek · 2017-11-16T17:38:58Z

Notes for the reviewer: The most important parts of the PR are SingleLanguageTranslationService and MultipleLanguageTranslationService, which implements TranslationService interface and can be replaceable.

The translation service is chosen depending on the config in CKEditorWebpackPlugin and passed alongside with the CKEditor5-specific env utils to the serveTranslation function, that takes care of the whole webpack compilation process.

…bundle.

ma2ciek · 2017-11-23T11:32:54Z

After above changes, the translation service should work according to the ckeditor/ckeditor5#624 (comment).

Steps to test:

Go to ckeditor5-dev
Switch branch to t/ckeditor5/624.
Run lerna bootstrap

Go to ckeditor5-dev/packages/ckeditor5-dev-webpack-plugin
Run npm link

Go to ckeditor5/packages/ckeditor5-utils
Switch branch to t/ckeditor5/624.

Go to ckeditor5/packages/ckeditor5-build-classic
Switch branch to t/ckeditor5/624.
Run npm link @ckeditor/ckeditor5-dev-webpack-plugin

Run npm run build

Now you should be able to play with options defined in ckeditor/ckeditor5#624.

pomek · 2017-11-23T11:38:56Z

You can avoid calling npm link. Use switch-to-dev-dev.sh script.

szymonkups · 2017-11-27T12:07:31Z

packages/ckeditor5-dev-webpack-plugin/lib/index.js

 	 */
 	constructor( options = {} ) {
 		this.options = options;
 	}

 	apply( compiler ) {
-		const { languages } = this.options;
+		if ( !this.options.language ) {


Maybe we could warn user about this in the constructor?

Hmm, we could even move more logic to the constructor despite calling serveTranslations(), but I'm not sure if this is necessary. TBH I'd move all config checks or none.

Ok, so lets warn here instead of moving it to the constructor.

szymonkups · 2017-11-27T12:11:56Z

packages/ckeditor5-dev-webpack-plugin/lib/index.js

+
+		if ( !additionalLanguages ) {
+			if ( this.options.outputDirectory ) {
+				console.warn( chalk.red(


What do you think about adding "Warning:" before the message and use yellow as in other warnings? To clearly state that it's not an error.

szymonkups · 2017-11-27T12:51:34Z

packages/ckeditor5-dev-webpack-plugin/lib/ckeditor5-env-utils.js

+ * @param {TranslationService} translationService
+ * @param {String} resource Absolute path to the resource.
+ */
+function maybeLoadPackage( cwd, translationService, resource ) {


Can we change name of this method to, for example getPathToPackage(). It would return proper path, or null. This way we will not pass TranslationService object here, just use the validated result outside the function.

szymonkups · 2017-11-27T12:52:42Z

packages/ckeditor5-dev-webpack-plugin/lib/ckeditor5-env-utils.js

+ * @param {String} resource Absolute path to the resource.
+ * @param {Array.<Object>} loaders Array of Webpack loaders.
+ */
+function maybeAddLoader( cwd, resource, loaders ) {


Same as above - rename to something like getLoader.

szymonkups · 2017-11-27T13:11:48Z

packages/ckeditor5-dev-utils/lib/translations/shortidgenerator.js

+		this._idNumber = 0;
+	}
+
+	getNextId() {


Please add docs to the method.

Reinmar · 2017-11-30T10:01:15Z

packages/ckeditor5-dev-env/lib/translations/collect-utils.js

@@ -17,7 +17,7 @@ const corePackageName = 'ckeditor5-core';

 const utils = {
 	/**
-	 * Collect translations and returns array of translations.
+	 * Collect translations and return array of translations.


Collects and returns.

ma2ciek added 7 commits November 9, 2017 10:46

WIP - Translation service v2.

3128d46

WIP - Aligning code to translation service v2.

4ffb882

Merge branch 'master' into t/ckeditor5/624

054454d

Added SingleLanguageTranslationService Improved API docs, tests.

232fc2b

Merge branch 'master' into t/ckeditor5/624

74a8873

Added TranslationService interface. Improved docs, serveTranslations …

873bea9

…and translation services.

Fixed file name and API docs.

94495e9

ma2ciek added the status:review- label Nov 15, 2017

ma2ciek added 2 commits November 15, 2017 16:00

Added tests for SingleLanguageTranslationService and `translateSour…

e7a11f7

…ce` files.

Added tests for MultipleLanguageTranslationService.

70acef0

ma2ciek force-pushed the t/ckeditor5/624 branch from df21254 to 70acef0 Compare November 15, 2017 16:01

ma2ciek added 7 commits November 15, 2017 17:43

Provided better error messages.

e2f0bcf

Fixed imports.

d3fcc64

Added test for translateSourceLoader.

2928db3

Added tests for CKEditorWebpackPlugin.

0272f50

Seperated ckeditor5-relative stuff from the main script. Added tests.

3e5785c

Fixed error message and API docs.

a5b697a

Improved API docs.

c77d021

ma2ciek added 3 commits November 22, 2017 10:29

Added translations: all option.

3ea3783

Merge branch 'master' into t/ckeditor5/624

60b28aa

Added missing tests for the languages: all option.

bf04f26

ma2ciek force-pushed the t/ckeditor5/624 branch from 4f34157 to bf04f26 Compare November 22, 2017 11:40

ma2ciek added 4 commits November 22, 2017 12:43

Added default param.

232b5d2

Enhanced TranlsationService. Made default language added to the main …

c2b8903

…bundle.

Fixed tests and added new ones.

ba2d8e4

Added missing test.

ba94e96

Added more API docs.

0060508

ma2ciek added 2 commits November 24, 2017 11:26

Added docs [skip ci].

e4b2c59

Changed options to language and additionalLanguages.

356cfab

ma2ciek force-pushed the t/ckeditor5/624 branch from 772d6bd to 356cfab Compare November 24, 2017 11:42

Fixed typos.

498e6a9

ma2ciek removed the status:review- label Nov 24, 2017

Fixed docs. [skip ci]

a5dcb91

ma2ciek force-pushed the t/ckeditor5/624 branch from eb788a6 to a5dcb91 Compare November 24, 2017 13:05

ma2ciek and others added 2 commits November 27, 2017 12:13

Merge branch 'master' into t/ckeditor5/624

d73fd5d

Small change to WebPack plugin description.

24efa6f

szymonkups suggested changes Nov 27, 2017

View reviewed changes

szymonkups and others added 4 commits November 27, 2017 14:26

Small doc fix in MultipleLanguageTranslationService.

8a5fdcf

Fixed errors and warnings in CKEditorWebpackPlugin`.

5761172

Added API docs. [skip ci]

ab9a0d5

Changed env utils so they don\'t change the TranslationService directly.

4fca55d

ma2ciek mentioned this pull request Nov 28, 2017

Align docs to the new Translation Service ckeditor/ckeditor5#694

Merged

ma2ciek added 2 commits November 29, 2017 11:53

Provided a 'verbose' flag.

d035f1a

Changed 'throwOnMissingTranslation' to the 'strict' option.

ab84ca0

szymonkups approved these changes Nov 29, 2017

View reviewed changes

ma2ciek added 2 commits November 29, 2017 14:50

Improved API docs.

e36663c

Merge branch 'master' into t/ckeditor5/624

78deab6

Reinmar reviewed Nov 30, 2017

View reviewed changes

szymonkups merged commit ee2a1d2 into master Nov 30, 2017

szymonkups deleted the t/ckeditor5/624 branch November 30, 2017 10:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement TranslationService v2 (ckeditor5-dev part) #314

Implement TranslationService v2 (ckeditor5-dev part) #314

ma2ciek commented Nov 15, 2017 •

edited

Loading

ma2ciek commented Nov 16, 2017

ma2ciek commented Nov 23, 2017 •

edited

Loading

pomek commented Nov 23, 2017

szymonkups Nov 27, 2017

ma2ciek Nov 27, 2017

szymonkups Nov 27, 2017

ma2ciek Nov 27, 2017

szymonkups Nov 27, 2017

szymonkups Nov 27, 2017

szymonkups Nov 27, 2017

szymonkups Nov 27, 2017

Reinmar Nov 30, 2017

Implement TranslationService v2 (ckeditor5-dev part) #314

Implement TranslationService v2 (ckeditor5-dev part) #314

Conversation

ma2ciek commented Nov 15, 2017 • edited Loading

Suggested merge commit message (convention)

Additional information

ma2ciek commented Nov 16, 2017

ma2ciek commented Nov 23, 2017 • edited Loading

pomek commented Nov 23, 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

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

ma2ciek commented Nov 15, 2017 •

edited

Loading

ma2ciek commented Nov 23, 2017 •

edited

Loading