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

Incompatible with hot module reload? #6

Closed
Danita opened this issue Jan 22, 2016 · 34 comments
Closed

Incompatible with hot module reload? #6

Danita opened this issue Jan 22, 2016 · 34 comments

Comments

@Danita
Copy link

Danita commented Jan 22, 2016

First of all, thank you for this work, 😄 it really looks a lot easier to use than react-intl which I find a little overkill for a simple app.

I have a translated view that goes like this:

import React from 'react';
import { translate } from 'react-i18next/lib';

const LoginForm = React.createClass({
    render() {
        const t = this.props.t;
        console.log(t); // <-- we should have the translation fn here
        return (
            <div>
                <h3>{ t('Sign in')}</h3>
                {/*...*/}
            </div>
        );
    }
});

export default translate(['login'])(LoginForm);

On first load and manual reload it works as advertised, displaying the translated message. The console.log(t) call returns something like function fixedT(key, options) {...}, but when I modify something on the component and thus triggering the hot reload module (Webpack), I get undefined and an Uncaught TypeError: t is not a function.

Is there any way to make react-i18next compatible with HMR?

Thanks!

@jamuhl
Copy link
Member

jamuhl commented Jan 24, 2016

will have a look on monday.

@jamuhl
Copy link
Member

jamuhl commented Jan 25, 2016

tried to reproduce on our env...but not having an issue with HMR...but might be we got a different configuration plus different versions of webpack.

Do you have a small project i could access to reproduce?

@Danita
Copy link
Author

Danita commented Jan 26, 2016

Hey Jan, thanks for taking a look at this. I have the latest versions of all dependencies, including Webpack. As of now, I replaced react-i18next for react-intl, but I think I still have the branch where I tried react-18next, so I could upload a repro case.

@jamuhl
Copy link
Member

jamuhl commented Jan 26, 2016

never mind...thanks for the reply...closing this for now.

@jamuhl jamuhl closed this as completed Jan 26, 2016
@sorahn
Copy link

sorahn commented Feb 3, 2016

I just ran into this same issue.

I'm using the code as a decorator

@translate(['common'])
class Header extends Component {
  static propTypes = {
    t: PropTypes.func
  };

  render () {
    const { t } = this.props

    return <h1>{t('hello_world')}</h1>
  }
}

Here's my i18n.js init.

import i18n from 'i18next'
import languages from './strings'

i18n
  .init({
    lng: 'en', // set dynamically on build
    // lngs: Object.keys(languages),
    fallbackLng: 'en',

    ns: ['common'],
    defaultNS: 'common',

    debug: true,

    interpolation: {
      escapeValue: false // not needed for react!!
    }
  })

Object.keys(languages).map(lang => {
  i18n.addResourceBundle(lang, 'common', languages[lang], true)
})

export default i18n

strings/index.js

import en from './en.json'
import de from './de.json'

export { en }
export { de }

export default { en, de }

strings/en.json

{
  "hello_world": "Hello World"
}

And I'm using the json-loader with webpack.

@sorahn
Copy link

sorahn commented Feb 3, 2016

FWIW This only seems to happen when saving the file with the decorator in it.

@jamuhl
Copy link
Member

jamuhl commented Feb 3, 2016

@sorahn can you try to exclude resources from json-loader?

@jamuhl jamuhl reopened this Feb 3, 2016
@sorahn
Copy link

sorahn commented Feb 3, 2016

@jamuhl do you mean make the strings .js files and objects?

      { test: /\.json$/, loader: 'json-loader', exclude: /node_modules/ },

Otherwise I'm not sure how to get the language files into the bundle with out that.

@jamuhl
Copy link
Member

jamuhl commented Feb 4, 2016

i wouldn't bundle the translations (or i would only bundle them in cases where you don't have a backend server -> standalone apps on mobiles). I would use the https://github.com/i18next/i18next-xhr-backend and let i18next load that json files from server.

Else you might define translations in js and add them by http://i18next.com/docs/api/#add-resource-bundle.

But anyway...when you bundle the translations keep in mind you bundle all languages which results in bigger bundle size.

@sorahn
Copy link

sorahn commented Feb 4, 2016

@jamuhl Yeah, we're not going to have a backend available to us, so we're going to just bake all 8 languages into the bundle. This bit loops through them and adds them all in.

Object.keys(languages).map(lang => {
  i18n.addResourceBundle(lang, 'common', languages[lang], true)
})

Everything itself is working great, the only issue comes from saving a file with the @translate decorator. When the HMR injects the newly created class, it doesn't have it's decorator function available.

@jamuhl
Copy link
Member

jamuhl commented Feb 5, 2016

Tried to reproduce:

added webpack-dev-server to example: https://github.com/i18next/react-i18next/tree/master/example#run-with-webpack-dev-server

added a class component with translate hoc as decorator: https://github.com/i18next/react-i18next/blob/master/example/app/components/View.js#L6

doing changes there all works well...can you provide a sample to reproduce this?

@jamuhl
Copy link
Member

jamuhl commented Feb 5, 2016

only difference i can see is i use a backend in the sample...but how that impacts the decorator to not get executed proper...idk

@sorahn
Copy link

sorahn commented Feb 5, 2016

We've got it at work right now, Let me try and strip it down and see if it still reproduces the problem. And I'll get it pushed somewhere.

@LucaColonnello
Copy link

Hey guys,
there's a solution.

Put this after i18next.init call.

if (module.hot) {
  module.hot.accept([assetsModule], () => {
    const res = require([assetsModule]);
    Object
      .keys(res)
      .forEach((lang) => {
        Object
          .keys(res[lang])
          .forEach((namespace) => {
            i18next.addResourceBundle(lang, namespace, res[lang][namespace], true, true );
          })
        ;
      })
    ;

    i18next.emit('loaded');
  });
}

Where [assetsModule] is the module that export the resources, used as resources in the initial option and also for the hot replacement.
You can also use i18next-resource-store-loader.

Complete Example:

import i18next from 'i18next'
const resources = require("i18next-resource-store-loader!../assets/i18n/index.js");

i18next
  .init({
    lng: 'en', // set dynamically on build
    fallbackLng: 'en',

    resources: resources,
    debug: true,

    interpolation: {
      escapeValue: false // not needed for react!!
    }
  });

if (module.hot) {
  module.hot.accept("i18next-resource-store-loader!../assets/i18n/index.js", () => {
    const res = require("i18next-resource-store-loader!../assets/i18n/index.js");
    Object
      .keys(res)
      .forEach((lang) => {
        Object
          .keys(res[lang])
          .forEach((namespace) => {
            i18next.addResourceBundle(lang, namespace, res[lang][namespace], true, true );
          })
        ;
      })
    ;

    i18next.emit('loaded');
  });
}

With this, every time you change a value into resources, the hot reloader replace all the namespaces with new values into i18next and emit loaded event.

Doing so, all the react component wrapped with the Translate component, will be re-rendered.

@jamuhl
Copy link
Member

jamuhl commented Feb 18, 2016

@LucaColonnello cool...if you don't mind i will add i18next-resource-store-loader to i18next.com under ecosystem...so others can find this too.

@LucaColonnello
Copy link

Yeah @jamuhl , I think it could be helpful to also add the solution I posted for the hot reloading in a FAQ or How To section..

Some users use hot reloading (all users indeed) and an Internationalization system has to manage all the users' needs.

@jamuhl
Copy link
Member

jamuhl commented Feb 18, 2016

@LucaColonnello agree. At our place we just have the translations on the server and use the xhr backend coming with i18next. That configuration works with hotreload too. Things only do not work when you start to require the translations and bundle them into build output.

so i will add some writeup on both alternatives. or we might even add another sample to the example folder in this repo (one using xhr backend is already done).

@LucaColonnello
Copy link

@jamuhl really good!!!!!

@Ganasist
Copy link

I'm using this great package and it's working fine, love it. I have one weird scenario...

Normally I use the @translate(['namespace']) decorator in ALL of my components, and it's been working fine. However, for one component (that is identical and in parallel with many others), this does not work. In all of the parallel components I import translate and use the decorate as above, I get 't' in the props and use it as usual...

BUT, I'm able to fix this one rogue component by reverting to this format..

export default translate(['login'])(LoginForm)

...and skipping the decorate pattern altogether. Any idea why this might be? Here's the error in the one component when I try to use the standard decorator approach (and which I never see in any other component):

Uncaught TypeError: Cannot read property 'loadNamespaces' of undefined - translate.js:55

@jamuhl
Copy link
Member

jamuhl commented Feb 18, 2016

@Ganasist seems like in that case somehow context.i18n is not set https://github.com/i18next/react-i18next/blob/master/src/translate.js#L14 ...but honestly no idea why

@jamuhl
Copy link
Member

jamuhl commented Apr 1, 2016

closing this...reopen if still having some issues..

@jamuhl jamuhl closed this as completed Apr 1, 2016
@hollanderbart
Copy link

I also have the same issue as @Ganasist. Get the error Uncaught TypeError: Cannot read property 'loadNamespaces' of undefined - translate.js:55
when I use the HOC translate method.

export default translate('common')(ComponentName)

When I don't include translate and only use the i18n, everything goes fine.

@jamuhl
Copy link
Member

jamuhl commented Apr 8, 2016

@hollanderbart still no idea how to track this one down....

@hollanderbart
Copy link

I'm getting this error when using Jest as my unit test framework. When I use the following setup for my i18next (HOC):

import { translate } from 'react-i18next'
render() {
const { t } = this.props
return(
<div>
<input type="text" value={this.state.filter} placeholder={t('common:filter.filterpalceholder')} onChange={this.handleFilterChange}></input>
</div> ) }
export default translate('common')(FilterRow)

I get this error in my Jest unit test:
- TypeError: Cannot read property 'loadNamespaces' of undefined at Translate.componentWillMount (node_modules/react-i18next/dist/commonjs/translate.js:57:15)

When I would replace my code to the direct i18n setup, with no import { translate }, everything goes perfect.

render() {
return(
<div>
<input type="text" value={this.state.filter} placeholder={i18n.t('common:filter.filterpalceholder')} onChange={this.handleFilterChange}></input>
</div> ) }

@jamuhl
Copy link
Member

jamuhl commented Apr 8, 2016

Cannot read property 'loadNamespaces' of undefined

there is no i18n on context --> that's what i18nextProvider does...for tests i would mock away the translated hoc...

or better not test the extended component but only the component:

export myComponent function(props) {...} <- test this one
export default translated('ns')(myComponent); <- not that

@hollanderbart
Copy link

I agree with you. The only thing is that I have trouble with mocking translate. When I use jest.mock('react-i18next'); I get the following error: TypeError: (0 , _reactI18next.translate)(...) is not a function because I use translate in export default translated('ns')(myComponent);

@hollanderbart
Copy link

I got it working. Finally.
Via this post: #3, I was mentioned this post: https://github.com/ghengeveld/redux/blob/a68768ff8d6a4fbe7b8f6a06a8cf9b99a54aefb0/docs/recipes/WritingTests.md#testing-decorated-react-components

Basically, because in order to test the undecorated component and not the decorated translate component, I had to export the component using: export class ComponentName extends Component { ... } next to the export default translate('common')(ComponentName) at the bottom.
Because I export the undecorated component next to the decorated component, I can import the undecorated component in my unit test using import { FilterRow } from '../components/filterRow'; instead of import FilterRow from '../components/filterRow';

@gnapse
Copy link

gnapse commented Sep 27, 2017

we just have the translations on the server and use the xhr backend coming with i18next. That configuration works with hotreload too.

@jamuhl I have a react app created with create-react-app, we're also using i18next with the xhr backend, and I'm starting to use HMR, which works fine for components, but when I change the translation files in the public/ directory, I'm not sure how I can make it work. Can you share what module.hot.accept(...) call you're making to make this work?

@jamuhl
Copy link
Member

jamuhl commented Sep 27, 2017

@gnapse you mean if you change something in the translation files you want the hmr to reload your page...not sure if that is possible...as those files are not part of the bundle.

I think the initial issue was a reload based on a change in the component...not translation files.

Personally we moved our translation to the service tier locize.com - so we do no manipulations of translations anymore inside the project repository.

Rather sure there is a way to trigger hmr doing some configurations to watch for the public folder too...but personally i prefer doing create-react-app init - so i might have less knowledge about webpack config...eventual this is a good question for stackoverflow?

@gnapse
Copy link

gnapse commented Sep 27, 2017

Thanks, I may indeed take this to SO. Just one last question: it has always seem weird to me that even when the translation files in the public/ directory are not part of the bundle, when I change them, the app running in dev mode automatically reloads the whole page, exactly as when I change something in the code inside src/.

@jamuhl
Copy link
Member

jamuhl commented Sep 27, 2017

hm...might trigger a reload in that case - than it's just the question of what is coming in module.hot.accept("i?!?!?" and from there trigger a reloadResources:

if (module.hot) {
  module.hot.accept("i?!?!?", () => {
   // then just
   i18next.reloadResources();
 });
}

If you got more feedback fro SO - i would be happy if you share your solution - would be a great add to documentation.

@gnapse
Copy link

gnapse commented Sep 27, 2017

Thanks for the i18next.reloadResources tip! That was a missing piece in my puzzle. I've yet to unravel if this is going through module.hot at all. If I find anything relevant I'll bring it back here.

@LucaColonnello
Copy link

LucaColonnello commented Sep 27, 2017 via email

@gnapse
Copy link

gnapse commented Sep 27, 2017

@LucaColonnello I believe your solution is not meant for when the XHR backend is being used. Or is it? For starters, what should I use in place of [assetsModule]? You say it is this:

Where [assetsModule] is the module that export the resources, used as resources in the initial option and also for the hot replacement.

But what module you refer to that export what resources? If you mean modules exporting your translations, when you use the XHR backend, you don't keep the translations in the bundle. They're loaded via ajax request when the app is loaded.

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

7 participants