Typescript

[nayaabkhan]

[ctavan]

@nayaabkhan I see you have started a Typescript-Rewrite.

I do have some interest in getting rid of the componentWillReceiveProps deprecation warning so I was wondering if you are palling to include that change in here (similar to #32)?

I'd also be happy to help, I was just wondering whether it made sense at all to supply a patch for this based on the current JavaScript develop branch or whether I should rather wait for you to finish the TypeScript rewrite?!

[nayaabkhan]

@ctavan I think it is better to patch the componentWillReceiveProps in a separate PR from the current develop branch. Feel free to do that, TypeScript migration isn't something that should block it.

The commit at https://github.com/nayaabkhan/react-polyglot/pull/32/commits/c5c6fac5a0decca393a3e84d3948d13c009a2a0d is pretty much all we need but maybe if we can trim it down to only the substitution of componentWillReceiveProps it would be easier to move it ahead.

[ctavan]

@nayaabkhan I just realized that in https://github.com/nayaabkhan/react-polyglot/commit/9210c448b07c5810b97a162e297dc100c3959036 componentWillReceiveProps was replaced by componentDidUpdate.

This means that with 0.6.0 the react deprecation warning is gone. I'm still a bit concerned whether componentDidUpdate is the right lifecycle method to update the polyglot instance (i.e. whether this will propagate the changes down the component tree before it has been re-rendered). Do you know if this has been tested?

Also, what was confusing me in the first place is that the current master branch of this repo does not even contain the latest release of this library (which I then found in develop), maybe either make the develop branch the default on github or go back to master?!

[nayaabkhan]

@ctavan Ah yes, I forgot to bring master up to speed with develop since my last pushes. I just did that, sorry for the confusion.

About componentDidUpdate, it is indeed something to be verified. I will look into it and if required, patch it.

[nayaabkhan]

@ctavan Thanks for pointing out about the componentDidUpdate, it was indeed causing the bug where the instance was not in sync with the new props. I have pushed a patch.

[ctavan]

Thanks! Which means we still need a different way of doing this in order to remain compatible with react>=17…

[soda0289]

@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 took this over to #38 to not further hijack this TypeScript pull request, let's continue the discussion over there.