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

feat: Cloud service providers & Translation.io #1107

Merged
merged 8 commits into from
Sep 7, 2021
Merged

feat: Cloud service providers & Translation.io #1107

merged 8 commits into from
Sep 7, 2021

Conversation

MichaelHoste
Copy link
Contributor

@MichaelHoste MichaelHoste commented Jul 27, 2021

Hello LinguiJS team,

This is a PR proposal that is open for suggestions.

Context

I'm the founder of Translation.io, a cloud service provider that helps localize applications and smooth the syncing process with translators. We decided to push best practices to keep developers and translators happy in the long run. We already support the Ruby on Rails and Laravel stacks with a decent success.

We tried several times to support JS/React but we didn't like the existing solutions back then. Then we found LinguiJS, and it was exactly what we were looking for all that time: GetText-like easy syntax with simple source code extraction and PO format.

You really did an amazing job! 😄

Proposal

We have decided to add Lingui as a supported framework. After a quick proof-of-concept demo and a discussion with @semoal, we thought it would be a good idea to create a new configuration option in Lingui for it to be able to work with any cloud service provider, and not only Translation.io

So the idea is to add something like this in the .linguirc:

"service": {
  "name": "TranslationIO",
  "apiKey": "abcdefghijklmnopqrstuvwxyz123456"
}

A new services folder has been created for the provider specific workflows.

I created a hook in the lingui-extract.ts process to be able to import the service by its name, it will be cleaner when there will be multiple services.

How it works

After a lingui extract, the service sends the content of all the PO files to Translation.io (or any other provider).

  • If it's the first time, the segments will be created on Translation.io (source & translation).
  • If it's not the first time, new segments will be created on Translation.io, and new translations will be sent back to your target POs.

The idea would be to just focus on your code, and never edit your PO files again!

Source text (usually English) will evolve in your code, and your translations will evolve on Translation.io and pushed back into your code at each extract (it works without conflict even if you work on multiple branches, with a process similar to obsolete and --clean).

Big advantages of a cloud provider is auto-translation, interpolation and HTML highlighting, (+ warning if missing), fine-grained collaboration roles, history of changes, automatic notification of translators, or ICU plural management (see below).

Demo

Here is a small demo where you can see the initialization process, the sync process and the translation interface with:

  • Variable interpolation & HTML tags management.
  • ICU Plural management with examples for French and Czech (Czech has 3 plurals).
lingui-test.mp4

Final notes

I'm convinced that a better symbiosis between LinguiJS and cloud solutions would be beneficial for the library popularity, for LinguiJS users, and of course for cloud solutions themselves.

If you want to test this PR, feel free to:

  1. Create an account in Translation.io.
  2. Create a Lingui project using this magic link https://translation.io/new?lingui
  3. Check the open-source checkbox and use https://github.com/lingui/js-lingui as link, it will provide you with a free unlimited access like any other open-source projects using Translation.io.

Any feedback would be of course greatly appreciated 🙂

@vercel
Copy link

vercel bot commented Jul 27, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/lingui-js/js-lingui/CeXcqRhFV4WrVe1B1q2EXCr8Ytbb
✅ Preview: https://js-lingui-git-fork-translation-translationio-lingui-js.vercel.app

@semoal
Copy link
Contributor

semoal commented Aug 30, 2021

I'll review the pull request along the week (have been on holiday for a time, had to disconnect) Michael and I'll release a new version with this feature introduced next Tuesday.

@MichaelHoste
Copy link
Contributor Author

Thanks a lot @semoal !
I hope you enjoyed your holidays.

Please let me know if you have any feedback or things that I could do to improve this PR.

@semoal semoal mentioned this pull request Sep 2, 2021
15 tasks
@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #1107 (1ae6d17) into main (ea00f55) will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1107      +/-   ##
==========================================
- Coverage   82.28%   82.08%   -0.21%     
==========================================
  Files          56       56              
  Lines        1677     1686       +9     
  Branches      458      462       +4     
==========================================
+ Hits         1380     1384       +4     
- Misses        173      177       +4     
- Partials      124      125       +1     
Impacted Files Coverage Δ
packages/conf/src/index.ts 79.74% <ø> (-1.03%) ⬇️
packages/cli/src/api/extractors/index.ts 55.55% <0.00%> (-13.68%) ⬇️
packages/macro/src/macroJs.ts 90.97% <0.00%> (ø)
packages/macro/src/macroJsx.ts 90.97% <0.00%> (ø)
packages/cli/src/api/catalog.ts 81.48% <0.00%> (ø)
packages/cli/src/api/compile.ts 96.15% <0.00%> (+0.15%) ⬆️
packages/core/src/context.ts 81.13% <0.00%> (+0.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea00f55...1ae6d17. Read the comment docs.

@MichaelHoste
Copy link
Contributor Author

I fixed the Linter warnings.

Actually, item.obsolete was a weird warning that was already ignored at several places like here: https://github.com/lingui/js-lingui/blob/main/packages/cli/src/api/formats/po.ts#L35

I have no idea where it comes from, or how to fix it (except using item['obsolete'] but that doesn't "fix" anything).


Actually, I'm sorry but there is still a warning when executing yarn extract or yarn compile with the new service option in the config file.

● Validation Warning:

  Unknown option "service" with value {"apiKey": "abcdefdummy123456789012345678901", "name": "TranslationIO"} was found.
  This is probably a typing mistake. Fixing it will remove this message.

Documentation: https://lingui.js.org/ref/conf.html

I don't know how to fix this, I did some changes here but that doesn't seem to have any effect.

What would be the correct way for me to improve this?

@semoal
Copy link
Contributor

semoal commented Sep 7, 2021

Also, another question after re-reading the code. Does your service only work with .po files? .json (aka minimal on Lingui) doesn't work?

If they don't work we should add an error clause where only allow running a given service for a given config.format.

@semoal
Copy link
Contributor

semoal commented Sep 7, 2021

You can replace item.obsolete with item['obsolete'], I don't know why they typed on TypeScript types that is a private method, so don't worry use it with [] and we're good.

About the warning, I'm not sure how are you testing it but usually, it's a bit tricky (I have to improve that asap), what I do is publishing the whole library to Verdaccio and install that published library on my projects to test that everything runs fine, on CONTRIBUTING.md there's a guide. Be careful about pending changes without committing because when releases it's like a real release and will change something that you must discard.

Anyways, I checked the conf files and everything is fine so that warning should disappear when released, so don't worry about that.

Also, not sure If you have enough time to write a small piece of documentation about this, but would be pretty cool. (If you don't have time don't worry we can ship docs at other time and another pull request much better.)

@MichaelHoste
Copy link
Contributor Author

Does your service only work with .po files? .json (aka minimal on Lingui) doesn't work?

It's only working with traditional PO files (not the po-gettext because I have a more fine-grained management of language plurals forms on the backend). But actually, since using a cloud provider prevents the manual management of translated files, that shouldn't be a problem for developers to leave the default format.

If they don't work we should add an error clause where only allow running a given service for a given config.format.

I'll do that directly inside the Translation.io service file, since other cloud providers may need another default format, or maybe be compatible with several of them. I'll keep you updated!

You can replace item.obsolete with item['obsolete'], I don't know why they typed on TypeScript types that is a private method, so don't worry use it with [] and we're good.

Done :-)

About the warning, I'm not sure how are you testing it but usually, it's a bit tricky (I have to improve that asap), what I do is publishing the whole library to Verdaccio and install that published library on my projects to test that everything runs fine, on CONTRIBUTING.md there's a guide. Be careful about pending changes without committing because when releases it's like a real release and will change something that you must discard.

First I used Verdaccio, but now I'm requiring local dependencies directly in my test app to speed up the process. I'll try again Verdaccio to have a cleaner environment and remove the warning.

Anyways, I checked the conf files and everything is fine so that warning should disappear when released, so don't worry about that.

Ok, great 😄

Also, not sure If you have enough time to write a small piece of documentation about this, but would be pretty cool. (If you don't have time don't worry we can ship docs at other time and another pull request much better.)

I already added this on my TODO list a few hours ago. If it's ok with you, I'll do that after the first release at the same time as the docs on Translation.io, it will be easier. I noticed that the docs generate automatically. It's great so we won't need to wait for another release.

Thanks again!

@MichaelHoste
Copy link
Contributor Author

MichaelHoste commented Sep 7, 2021

I added the error when the Translation.io service was configured with a format different than po.

I can also confirm that the warnings disappeared with Verdaccio 😄

@semoal semoal changed the title Cloud service providers & Translation.io feat: Cloud service providers & Translation.io Sep 7, 2021
@semoal semoal merged commit cbc87b5 into lingui:main Sep 7, 2021
@semoal
Copy link
Contributor

semoal commented Sep 7, 2021

There we go mate, I'll release a new version with this feature and all these bug fixes #1128 in a few hours

@MichaelHoste
Copy link
Contributor Author

Thanks a lot for your help @semoal!

I really hope this feature will benefit both Lingui and Translation.io, and that other cloud providers will be able to add their synchronization processes in the future. For big applications with lots of evolving text in dozens of target languages, it's a must-have.

@JSteunou
Copy link
Contributor

@MichaelHoste thank you for this feature and your SaaS product! I just came across this new feature reading the linguijs doc, will definitely play with it!

@MichaelHoste
Copy link
Contributor Author

@JSteunou I'm glad that people are interested in this integration 😉

Feel free to contact me directly if you need some help or if you find any issues with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants