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

Force all languages refresh #17

Closed
PeterKottas opened this issue Sep 5, 2020 · 9 comments
Closed

Force all languages refresh #17

PeterKottas opened this issue Sep 5, 2020 · 9 comments

Comments

@PeterKottas
Copy link

Lovely piece of code! Works like a charm but I noticed one problem. If I understand correctly, the code in loader.js works in a way where it only triggers once even if multiple updates are picked up. We use https://github.com/gilmarsquinelato/i18n-manager at my company (amazing tool) but the problem is it "re-saves" all translation files on save. This breaks the experience because it reloads random language (I assume the one saved first).

It should be very easy to fix this by providing options to applyClientHMR, something like:

{
  forceAllReload: boolean
}

and then based on this setting not providing the parsed arguments to the reloadResources i18n function.

@PeterKottas PeterKottas changed the title Multiple files saved causes an issue when figuring out the saved language Force all languages refresh Sep 5, 2020
@PeterKottas
Copy link
Author

Naturally, the behavior is the same for the namespace. In my case, I have 2 languages with 4 namespaces each. Upon saving, it just randomly refreshes one namespace on one language.

@PeterKottas
Copy link
Author

PeterKottas commented Sep 5, 2020

I was digging around for a quick solution and put together something that works surprisingly well. I'm putting it here for you to evaluate. In my case, this basically removes the need for the lib altogether.

const context = require.context('./assets/locales/', true, /\.json$/);
function importAll(r: __WebpackModuleApi.RequireContext) {
  r.keys().forEach(r);
}
if (module.hot) {
  importAll(require.context('./assets/locales/', true, /\.json$/));
  module.hot.accept(context.id, function() {
    i18n.reloadResources().then(() => {
      i18n.changeLanguage(i18n.languages[0]);
    });
    importAll(require.context('./assets/locales/', true, /\.json$/));
  });
}

Maybe you can wrap this in a function and expose it as a simplified solution? (With this, you don't need the plugin, that can be handy when working with create-react-app or other libs where access to webpack config is somewhat complicated). Just thinking out loud here.

@felixmosh
Copy link
Owner

Hi @PeterKottas, thank you for reporting this issue 🙏🏼.

From your code snippet you are putting the entire translation json into the webpack bundle, when I've developed the plugin, I wanted to avoid this, there is no need for parsing & re-evaluating them as part of the dependency tree.

This line should reload only the current language.
I want to "see" the DX when using i18next-manger.

@PeterKottas
Copy link
Author

Thanks for a quick reply mate. Yeah, you're absolutely right, the files end up in the bundle with this. But TBH, in dev, I am not too fused about that. Especially for smaller apps. But let's forget about my hacky workaround. With regard to the issue in this lib, it's actually this that causes only the language parsed from the updated file to reload. Or to be more precise it's the 2 lines in conjunction.

The main issue is caused by watch trigger that only notices the first file that get's changed. You can actually try this yourself without manager. If you save 2 files really quickly one after another, I believe you will see the same issue. The thing is, I am not sure, but that might be a webpack thing. The easiest would therefore be to force all languages to refresh. That means calling reloadResources without params and then change lang. Probably conditionally based on some config.

@felixmosh
Copy link
Owner

I will try to reproduce it, it is interesting, because the loader is dependent on the entire translation folder, but probably get re-run only with one of them.
Looks like this is related to https://github.com/felixmosh/i18next-hmr/blob/master/lib/plugin.js#L32

@felixmosh
Copy link
Owner

As I've suspected, the lib is taking the first file that was changed.

I will fix it at the evening, thank you 🙏🏼

@PeterKottas
Copy link
Author

I will try to reproduce it, it is interesting, because the loader is dependent on the entire translation folder, but probably get re-run only with one of them.
Looks like this is related to https://github.com/felixmosh/i18next-hmr/blob/master/lib/plugin.js#L32

Good catch! That definitely might be causing that.

As I've suspected, the lib is taking the first file that was changed. I will fix it at the evening, thank you 🙏🏼#

Awesome, thanks a lot mate! Handy little lib you got here. The code is also quite nice, I like how you solved the loader stuff, learned something today ;)

@felixmosh
Copy link
Owner

@PeterKottas can you confirm that v1.6.0 solves the issue for you?

@PeterKottas
Copy link
Author

Tested, all works as expected now. Well done mate! Thanks.

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

No branches or pull requests

2 participants