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

Avoid setState while react is rendering #1165

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

rmedaer
Copy link
Contributor

@rmedaer rmedaer commented Aug 26, 2020

As documented in my comment about issue #1124, this fix should avoid setState while react is rendering.

See #1124 for more details.

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • documentation is changed or added

As documented in i18next#1124 (comment)
this fix should avoid `setState` while react is rendering.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.672% when pulling b7b2869 on rmedaer:fix/suspense-set-state into a144e14 on i18next:master.

@rmedaer
Copy link
Contributor Author

rmedaer commented Aug 26, 2020

@jamuhl, I don't know how to fix the coverage issue, it's (I guess) a side effect which (according to the report) is not in src/useTranslation.js. If you really need me to fix the coverage for this patch, I would enjoy some help.

Kind regards,

@jamuhl
Copy link
Member

jamuhl commented Aug 26, 2020

@rmedaer not needed...will check PR this asap...thank you

@jamuhl jamuhl merged commit dbd54d5 into i18next:master Aug 27, 2020
@jamuhl
Copy link
Member

jamuhl commented Aug 27, 2020

published in [email protected]

Thank you for figuring this out and providing the PR. Hopefully this will not break any side cases - but looks safe in my tests.

@rmedaer
Copy link
Contributor Author

rmedaer commented Aug 27, 2020

published in [email protected]

Thank you.

Hopefully this will not break any side cases

Let me know if you find related issue.

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.

3 participants