Activity Log: Fix data layer on credentials save error

[tyxla]

It appears in the data layer where we save the credentials we're currently using translate() as an exported module, but that won't work with it, because that way it loses the this reference to the I18N singleton. So if we try to save wrong credentials, users never see error notices, instead they get the following JS error:

To fix this, we simply should directly use i18n.translate() in all these locations.

Changes proposed in this Pull Request

• Properly use i18n.translate() instead of { translate } = i18n; translate().

Testing instructions

• Go to http://calypso.localhost:3000/settings/security/:site where :site is a Rewind-enabled site that doesn't support auto credentials provisioning.
• Input some wrong credentials and click the save button.
• Wait a little bit.
• Make sure you properly get the error notice instead of getting an unexpected JS error.

Context

Discovered while building a guided tour for #32152

[matticbot]

Test live: https://calypso.live/?branch=fix/rewind-credentials-data-layer-errors

[dmsnell]

Thanks @tyxla for the fix! Do you know when the i18n behavior changed? This worked fine when we rolled it out.

[tyxla]

I'm not sure what happened here @dmsnell. AFAIK, i18n-calypso has always had a singleton under the hood. I haven't investigated why and how this got wrong, but I assume it probably has something to do with the move of i18n-calypso to calypso.

[dmsnell]

I assume it probably has something to do with the move of i18n-calypso to calypso

Singleton isn't a problem. It appears like when we brought it into Calypso we also changed from CommonJS style exports to a module-style export.

In this module we're importing i18n from 'i18n-calypso' which in CommonJS gets the full export object itself but in modules gets the default export. Well the default export is the i18n object while the CommonJS export object includes a bound version of translate()

In this code when we call translate() in the CommonJS world we're making a function-call that was a bound method on an object and it retained its this binding by means of the bind(), but in the default export we're calling the unbound method on the i18n object directly, disconnecting the this

ironically if we had done this…

import { translate } from 'i18n-calypso';

…then I believe that everything would have continued to work because the new module export { translate } is also a bound version of the method itself.

cc: @blowery

Although we have more than a 100 instances of import i18n from 'i18n-calypso' it is only this module that then destructures translate() from the imported name; meaning that I think there should be no other cases where the change in export syntax introduced regressions.

---

With a bit of digging it looks like a more appropriate refactor would use the useTranslate() React hook, introduced in #31043