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

Add Support for Static Catalog #432

Merged
merged 1 commit into from
May 24, 2020
Merged

Add Support for Static Catalog #432

merged 1 commit into from
May 24, 2020

Conversation

einfallstoll
Copy link
Contributor

In order to webpack i18n I want to statically set the catalog. This fixes #262.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 97.732% when pulling 0e07333 on einfallstoll:static-catalog into a1fa83b on mashpie:master.

@einfallstoll
Copy link
Contributor Author

I could successfully webpack i18n:

webpack.config.js:

module.exports = {
  /* ... */
  node: {
    fs: 'empty'
  }
};

app.js:

import i18n from 'i18n';

i18n.configure({
  staticCatalog: {
    de: require('../../locales/de.json'),
  },
  defaultLocale: 'de',
  updateFiles: false,
  objectNotation: true,
});

i18n.setLocale('de');

i18n.__('something'); // etwas

@mashpie
Copy link
Owner

mashpie commented May 10, 2020

This looks promising - if only I had seen that at start of weekend. Will quickly review and merge next week...

@einfallstoll
Copy link
Contributor Author

I'm quite happy with the solution and it seems to work as intended. However, I noticed some strange behaviour during testing. If you replace en with de, the tests will pass when they are exclusively tested (npm test test/i18n.configureStaticCatalog.js), however, they will fail if all test cases run (npm test). Because in i18nGetCatalog(object, locale) the language will suddenly fall back to en for some reason. I believe previous testcases override global settings causing the code to use a wrong language.

@mashpie
Copy link
Owner

mashpie commented May 10, 2020

yap - main problem is that i18n currently still is tested as singleton. So tests with different settings may (...will) introduce side effects. One option to come around that singleton is to clear require cache. But there is another PR open for support of real instances. Plus I am planing a rewrite for 1.x branch.

So for now I want to stick to existing tests as a POC for integration until code refactor is finished. And after that tests will get a refactor to get isolated with instances. Well, that's the plan...

Meanwhile I do carefully reviews even if tests pass... :)

@einfallstoll
Copy link
Contributor Author

@mashpie Any update on this?

@mashpie
Copy link
Owner

mashpie commented May 24, 2020

honestly... just entered gihub url in chrome a second ago too merge (I saw you comment appearing on second screen while checking commits)

@mashpie mashpie merged commit 8aa57b8 into mashpie:master May 24, 2020
@mashpie
Copy link
Owner

mashpie commented May 24, 2020

and released to npm as 0.10.0

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.

require JSON instead of reading from FS
3 participants