search

Cropster / ember-l10n

A GNU gettext based localization workflow for Ember
MIT License
15 stars 7 forks source link

Use asset map for fingerprinting #27

Closed mydea closed 6 years ago

mydea commented 6 years ago

This PR replaces the usage of our own, custom fingerprinting mechanism, to instead use the regular fingerprinting provided by ember-cli.

In order to fetch the fingerprinted files, I used ember-cli-ifa. This fetches the asset map, which contains a mapping of files to their fingerprinted counterparts, and then uses this to fetch the resources.

This allows us to get rid of the utils/l10n-fingerprint-map.js file, and to not generate a new .json file for each new version. This also makes version control of translation files much better.

The downside of this is that it requires a little bit of configuration (to ensure .json files are fingerprinted), and it requires consumers of the addon that used the 'old' fingerprinting to move their files from e.g. /public/assets/locales/1234/en.json to /public/assets/locales/en.json. This requires this to be a major version bump, if released. (--> 2.0.0). I outlined all the upgrade steps in the readme, as well as adding all the copnfiguration options & requirements.

Additionally, this PR also fixes the regex to check the plural forms - this was incorrect, and resulted in it rejecting all - correct! - plural forms, even in the dummy app.

Finally, this PR also adds a skip-check-install option to all CLI commands. On windows, it sometimes doesn't find the correct dependencies, even if they are installed, and will then always install them again. This option allows you to skip that check.

mydea commented 6 years ago

@arm1n:

  1. On windows, for me it doesn't seem to recognize the installed dependencies. So it tries to reinstall them every time, which takes some time and is pretty annoying.
  2. With this PR, it will not create new files anymore, but always use e.g. locales/en.json. So there will be no more multiple files - they will only be fingerprinted in the build output!
arm1n commented 6 years ago

Okay great news. Concerning the skip-check-install - it's likely that the program detection via type ${bin} >/dev/null 2>&1 doesn't work on windows / power shell. Maybe we can find a way to catch all OS with the commands. I guess it always tries to reinstall gettext, right?

mydea commented 6 years ago

Yes, probably! I'll create a dedicated issue for that - I think having the option to skip the install check is fine anyhow, to catch any remaining issues. But of course it would be even nicer to have this work on Windows as well :)

arm1n commented 6 years ago

Super, thanks a lot!