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

Implement translation service (i18n) v2 #624

Closed
Reinmar opened this issue Oct 20, 2017 · 18 comments
Closed

Implement translation service (i18n) v2 #624

Reinmar opened this issue Oct 20, 2017 · 18 comments
Assignees
Labels
type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 20, 2017

Followup of #387

In #387 we didn't manage to implement a webpack plugin which would be able to create translation files for CKEditor. E.g. you can see this in builds – there's a single build/ckeditor.js file which contains English translations. If you'd like to have the editor in Polish you need to go through the entire Custom builds guides (see also Setting UI language). This is bad. Really bad.

What we need is that each build will contain the following files:

build/
    ckeditor.js
    lang/
        en.js
        pl.js
        ...

To enable the editor with a Polish interface you'll need to load two files (e.g. using script tags):

<script src="<path-to-build>/build/ckeditor.js"></script>
<script src="<path-to-build>/build/lang/pl.js"></script>

And initialize it:

ClassicEditor.
    create( {
        language: 'pl'
    } )
    ...

By default, if not specified, the editor will be initialised with English interface (cause that localization is automatically bundled in ckeditor.js).

Implementation

It'd be best if all this was handled by the CKEditorWebpackPlugin that we use for builds:

new CKEditorWebpackPlugin( {
	language: 'en',
	buildAlso: 'all' // or e.g.: [ 'pl', 'en' ]
} ),

If someone wants to build just a single language and have the most optimised build she/he can do it like this:

new CKEditorWebpackPlugin( {
	language: 'pl'
} ),

From CKEditor's code perspective, the language files will look something like this:

CKEDITOR_TRANSLATIONS.add( 'pl', {
    bold: 'Pogrubienie'
} )

And this will use the global API exposed by the https://github.com/ckeditor/ckeditor5-utils/blob/master/src/translation-service.js (basically, the add() function needs to be exposed).

If it won't be possible to implement this with a webpack's plugin it can also be done independently from webpack – by an external tool. Every build has its bin/build-ckeditor.sh file anyway so we can add more scripts there. However, this will need to be somehow synchronised with the settings passed to CKEditorWebpackPlugin().

One more thing to notice is that when that plugin is configured to build just one language it could use the current method which is to replace t( 'Bold' ) calls with e.g. t( 'Pogrubienie' ). When building for multiple languages such calls need to be replaced with t( 'some-key' ) (see the old ticket for more details about this) and this some-key needs to be passed to translation services (so a different translation service is need to be used – cause right now t() function returns its first param).

However, for simplicity, we could always replace t() with t( 'some-key' ), even when building for a single language. Then, it'd be just a matter of feeding the translations.

@Reinmar Reinmar added candidate:1.0.0 type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Oct 20, 2017
@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 9, 2017

I've got some issues that came to my mind recently, so I want to clarify few things.

Do we want to create hashes for translation keys (#387 (comment) ckeditor5-utils/src/translation-service part) and use them later? I'm not sure if your some-key means hash or translation string?

Do we want to support creating multiple chunks e.g. through the DllPlugin? We should be able to get all dependencies for the given asset, but I'll check it to be sure.

How we want to support multiple instances of the editor? Because we can't register one CKEDITOR_TRANSLATIONS.add functions when there're multiple editors running. We might:

  1. Register multiple add functions and switch CKEDITOR_TRANSLATIONS.add() to CKEDITOR_TRANSLATIONS.instances.forEach( instance => instance.add() ). Do each add function will merge new translations to the old ones for each editor's instance.

  2. Register one add function per editor that'd be overriding the previous one. So the translation files have to added be just after the editor.

We should be able to provide an option to specify the destination directory for the translation files because we need to fit many dev environments. I'd propose the 'languageDirectory' option which defaults to 'output'.

I'd try to write everything inside the webpack plugin, so the user won't need to execute anything. I'm testing https://webpack.js.org/api/compilation/#additional-assets-async and it should fit our needs.

I'm not sure how do we want to change editor's language at runtime. For now, the language for the UI is set in the config, so the user needs to rebuild everything either way.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 9, 2017

cc @szymonkups

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 10, 2017

Do we want to support creating multiple chunks e.g. through the DllPlugin? We should be able to get all dependencies for the given asset, but I'll check it to be sure.

Yes, it's possible to generate e.g. few 'en.js' translation files for few chunk during the compilation. Like in the original proposal:

Let's say, that we want to split a package to some preset X and packages Y and Z.
This will make for 3 files – x.js, y.js, z.js.

The idea is that each files will have an accompanying language(s) files defining a translations needed for this file. So there will be:

x.js, x-pl.js, x-en.js, ...
y.js, y-pl.js, y-en.js, ...
z.js, z-pl.js, z-en.js, ...

But it's easy to guess, that this is going to be complicated with the support for multiple editors etc.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 10, 2017

I'll sum up our talks:

Do we want to create hashes for translation keys (#387 (comment) ckeditor5-utils/src/translation-service part) and use them later? I'm not sure if your some-key means hash or translation string?

Yes, we do. Hashes as they resolve the problem with having same translation strings for different contexts. (the translation string's context is defined by the package name, e.g. ckeditor5-core).

Do we want to support creating multiple chunks e.g. through the DllPlugin? We should be able to get all dependencies for the given asset, but I'll check it to be sure.

Not necessarily, but we might need it at some point in the future.

How we want to support multiple instances of the editor?

Few editors instances should be compiled at the same time, so it's solved by the translation-service singleton. We have only one global CKEDITOR_TRANSLATIONS.add function.

We should be able to provide an option to specify the destination directory for the translation files because we need to fit many dev environments. I'd propose the 'languageDirectory' option which defaults to 'output'.

This is ok, as it's up to the developer, where these files should land.

I'm not sure how do we want to change editor's language at runtime. For now, the language for the UI is set in the config, so the user needs to rebuild everything either way.

We don't need it at runtime, but we should be able to change the editor's language by adding only one language file. With many language files, the editor's language should be specified by the config.lang configuration. English should be the default one.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 10, 2017

It should be noted that no two translation strings across the whole project can't share the same name. If they mean the same, they should be placed in the core package. Otherwise, the rarer string should contain [context: someContext] part, which will be cut off during the translation process.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 15, 2017

The part I haven't mentioned earlier is the error handling. I made it configurable by providing the throwErrorOnMissingTranslation option. By default, if the translation for some phrase is missing, the original translation key overrides it and the error is logged to the console during the compilation. With the above option enabled, the whole compilation is stopped.

So for now, I'd introduce the 4 plugin options:

  • languages - target languages
  • outputDirectory - output directory for the emitted translation files, relative to the Webpack's context, 'lang' by default.
  • optimizeBuildForOneLanguage - optimizes build for one language (directly replaces translation keys with the target language's translated strings), webpack doesn't emit any language file
  • throwErrorOnMissingTranslation - make plugin throw when the translation is missing

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

How should builds behave after that change? Should they translate code directly into the bundle, like before? Or should they emit files?

@Reinmar
Copy link
Member Author

Reinmar commented Nov 21, 2017

The answer is in the following piece of my initial idea (although, it's a bit hidden :D)

It'd be best if all this was handled by the CKEditorWebpackPlugin that we use for builds:

new CKEditorWebpackPlugin( {
	language: 'en',
	buildAlso: 'all' // or e.g.: [ 'pl', 'en' ]
} ),

ckeditor.js should come with a built-in English (so you can enable the editor by just loading ckeditor.js without any additional file), but there should be also other languages available.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

Hmm, but after the translation keys are replaced with the corresponding translations it's impossible to change those translations later. We're losing the contexts.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

We might end up with multiple 'Anuluj' from'Cancel [context: 1]' and 'Cancel [context: 2]'. So it won't be possible to translate 'Anuluj' to another language later.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

It'd be possible if we'd save the context in the translations.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

Note that we wanted to replace translation keys with hashes for optimizing the weight of the editor and translation files.

@Reinmar
Copy link
Member Author

Reinmar commented Nov 21, 2017

I'm not sure whether I understand what you mean, but saving the context in translations can be done. t() may strip that later on (I think that it was part of our initial idea).

Also, I think thee we agreed to ignore duplicated texts (when one text appears in two contexts) for now. So perhaps we can workaround this "temporarily" in a different way.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 21, 2017

So maybe I misunderstood the idea with ids/hashes. Because now I don't have an idea how they could be used and what's their purpose.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 23, 2017

EDITED

After F2F talks, we agreed, that we should go with such solution:

  • Workflow:

One entry point (or to be precise one output JS file):

  • additionalLanguages not set -> build optimized version
  • additionalLanguages set –> language will be built into the main bundle (e.g. ckeditor.js)

Multiple output JS files

  • additionalLanguages not set -> build optimized version
  • additionalLanguages set –> emit all translation files separately and warn user, that he needs to load at least one translation file manually to get editor working

Translation files will be emitted in the outputDirectory or lang directory if it's not set.

@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 24, 2017

I changed a little bit naming as the option I've proposed provided more problems than it solved.

So I added language [required] and additionalLanguages options instead of the languages and defaultLanguage.

Cases:

  • what if the defaultLanguage is not listed in languages
  • what if the defaultLanguage is empty and languages set to all?

So it should be easier to undestand and maintain such a config.

szymonkups added a commit to ckeditor/ckeditor5-utils that referenced this issue Nov 27, 2017
Other: Aligned code to the new Translation Service (ckeditor/ckeditor5#624).
@ma2ciek
Copy link
Contributor

ma2ciek commented Nov 27, 2017

Let's say, we're setting up the main language and set some additional languages. What if one of the translations for some additional languages misses? Should we have the fallback to the main language or English? Should it be configurable?

cc @Reinmar

@Reinmar
Copy link
Member Author

Reinmar commented Nov 28, 2017

Good question. I think that it doesn't need to be configurable. In CKE4 missing translations default to English. I think it'd be good to keep this behaviour so it needs to happen during building.

szymonkups added a commit to ckeditor/ckeditor5-dev that referenced this issue Nov 30, 2017
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.
@Reinmar Reinmar added this to the iteration 14 milestone Nov 30, 2017
szymonkups added a commit to ckeditor/ckeditor5-ui that referenced this issue Nov 30, 2017
Other: Aligned code to changes (`config.lang` to `config.languages`). Part of the Translation Service v2 (ckeditor/ckeditor5#624).
szymonkups added a commit to ckeditor/ckeditor5-heading that referenced this issue Nov 30, 2017
Other: Aligned code to changes (`config.lang` to `config.languages`). Part of the Translation Service v2 (ckeditor/ckeditor5#624).
szymonkups added a commit to ckeditor/ckeditor5-build-classic that referenced this issue Nov 30, 2017
Other: Aligned build and `webpack.config.js` to the new Translation Service (ckeditor/ckeditor5#624).
szymonkups added a commit to ckeditor/ckeditor5-build-inline that referenced this issue Nov 30, 2017
Other: Aligned build and `webpack.config.js` to the new Translation Service (ckeditor/ckeditor5#624).
szymonkups added a commit to ckeditor/ckeditor5-build-balloon that referenced this issue Nov 30, 2017
Other: Aligned build and `webpack.config.js` to the new Translation Service (ckeditor/ckeditor5#624).
szymonkups added a commit that referenced this issue Nov 30, 2017
Docs: Aligned docs and snippets to the Translation Service v2. Closes #624.
JDinABox pushed a commit to JDinABox/ckeditor5-build-markdown that referenced this issue Sep 6, 2021
Other: Aligned build and `webpack.config.js` to the new Translation Service (ckeditor/ckeditor5#624).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests

2 participants