Get rid of deprecated componentWillReceiveProps

[ctavan]

componentWillReceiveProps is deprecated and will be removed in React 17.

It's currently being used to update the internal node-polyglot instance when the <I18n> component's props change:

react-polyglot/src/i18n.js

Lines 22 to 30 in db80a63

	componentWillReceiveProps(newProps) {
	if (newProps.locale !== this.props.locale) {
	this._polyglot.locale(newProps.locale)
	}
	if (newProps.messages !== this.props.messages) {
	this._polyglot.replace(newProps.messages)
	}
	}

We need to find a future-proof way of updating the internal node-polyglot instance.

[ctavan]

From @soda0289 in #35 (comment) :

@ctavan @nayaabkhan

The issue with componentDidUpdate is that although we do update the polyglot instance it only happens after we have rendered.

Is there any reason why we cant just do the check inside render().

Something like

if (this.polyglot.currentLocale  !== this.props.locale) {
  this.polyglot.locale(this.props.locale);
}

That way we update the instance then create a new bound function that would trigger a re render of the Context consumers.

I can work on this change as well since we have run into issues where changing locale wont trigger re render.

**EDIT: The workaround we use now is to pass in a key prop for <I18n /> component that uses the locale.
<I18n key={locale} locale={locale} />

[ctavan]

@soda0289 I believe your suggestion of updating the polyglot instance in the render-method should work well for props.locale since by comparing the string this operation can be made idempotent and efficient.

I'm not so sure about the messages though: When calling this.polyglot.replace() the messages object is copied internally by node-polyglot so we cannot (shallow) compare props.messages with some value inside the node-polyglot instance.

At a first glance I would assume that what we want is the memoize-one solution from
https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#what-about-memoization

What do you think?

[soda0289]

I think memoizing the input props is the correct solution.

If we use the new react hooks they offer useMemo() which would work well in this siutaion but we would have to convert the component class into a component function to use hooks.

function I18n({children, locale, messages}) {
   const translate = React.useMemo(() => {
       this._polyglot.locale(this.props.locale)
       this._polyglot.replace(this.props.messages)

       return this._polyglot.t.bind(this._polyglot)
   }, [locale, messages]);

    return (
      <I18nContext.Provider value={translate}>
        {React.Children.only(children)}
      </I18nContext.Provider>
    )
  }
}

If we used memoize-one would it just be something like:

  function render() {
    const getTranslate = memoizeOne(
      (locale, messages) => {
        this._polyglot.locale(this.props.locale)
        this._polyglot.replace(this.props.messages)

        return this._polyglot.t.bind(this._polyglot)
      }
    );
   const translate = getTranslate(locale, messages);

    return (
      <I18nContext.Provider value={translate}>
        {React.Children.only(children)}
      </I18nContext.Provider>
    );
  }

Would it be ok to do a shallow comparsion of messages object. For my use case I always load the messages from a json file so the object never changes. Which would allow for shallow or "===" comparison.

[ctavan]

I believe we should do a shallow comparison only (and probably document this). Anything else could result in serious performance problems.

[soda0289]

@ctavan @nayaabkhan Any preference between using the react hooks with function component or should we be using the class component and bring in the memozineOne dependency?

[ctavan]

I‘m personally a big fan of hooks and would favor it.

We just need to make sure to adjust the react peerDependency accordingly and release the new version of this library as 0.7.0

[ctavan]

Speaking of hooks: It would then of course also be nice to have a const [t] = useTranslate(); hook instead of having to continue to use the HOC. 😉 Maybe as an additional API?

[soda0289]

@ctavan We added a hook already #22

Much nicer API then HOCs. Especially with typescript typing.

[ctavan]

Whoops, of course! Sorry for missing out on that one!

[nayaabkhan]

I think it would be fine to move our React peerDependency ahead to 16.8.0 and use the useMemo hook internally. In any case, people who aren't looking to migrate to a newer version of React can keep using the current version of react-polyglot without issues.

[ctavan]

Cool!

@soda0289 are you planning to make a pull request? Just so we don’t work both on it at the same time.

[soda0289]

@ctavan Yes I can work on this today or tommrow.

[puredazzle]

Hi! Any updates on this issue?

[nayaabkhan]

@puredazzle Hi! I will check the PR #39 that resolves this issue in the coming days.

[nayaabkhan]

Released v0.7.0 🎉