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

fix: prevent race condition between i18next causing translation to fail #341

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

Akarshit
Copy link
Contributor

@Akarshit Akarshit commented Feb 3, 2021

Resolves #309, #300
Impact: major
Type: bugfix

Issue

One of the flow that was causing this problem was:

  1. Before getting shop, we load "en" translations.
  2. The page is rendered.
  3. Now we get the shop, we again try to load the "en" translation in the same object.
  4. Before the object is ready React is re-rendering the components and the translations are all failing.
  5. The object loads, but the translations are already empty.

Note this is one possible flow, since the stuff is happening is async more could be possible.

Solution

Don't load the translations again until the language is changed.

Breaking changes

None exptected

Testing

  1. Start the platform.
  2. Visit admin, refresh the page a bunch of times. Now every-time the correct translations should load.

@Akarshit
Copy link
Contributor Author

Akarshit commented Feb 3, 2021

Just saw the failing lint, will update the code tomorrow.

@Akarshit Akarshit force-pushed the fix-309-akarshit-traslation-loading branch from 2fd7d65 to 5de3bcd Compare February 3, 2021 15:04
@Akarshit Akarshit force-pushed the fix-309-akarshit-traslation-loading branch from 5de3bcd to ccff1c2 Compare February 15, 2021 17:57
@Akarshit Akarshit changed the title (bugfix): prevent race condition between i18next causing translation to fail fix: prevent race condition between i18next causing translation to fail Feb 15, 2021
@Akarshit Akarshit requested review from a team and removed request for focusaurus and jrw421 February 15, 2021 17:58
Copy link
Contributor

@MohanNarayana MohanNarayana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix!

@Akarshit Akarshit merged commit f7ee61f into trunk Feb 15, 2021
@Akarshit Akarshit mentioned this pull request Feb 15, 2021
3 tasks
@Akarshit Akarshit mentioned this pull request Mar 3, 2021
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

Successfully merging this pull request may close these issues.

React variables are visible
2 participants