react-localize-redux
react-localize-redux copied to clipboard
Fix issue where new instances of translations and options are created in localizeReducer on every render
Fixed that the state changes on any action which is passed into the localizeReducer.
The problem was that the state was recreated on every localizeReducer call. The reducers for each of the states properties work correctly and only touches the state, when an action is modifing part of the state.
I added a unit tests and ran flow and all other tests. Unfortunately I got errors on both right after checking out the current master.
@ryandrewjohnson are you still maintaining the repository? I fix an issue and wanted to ask if you can review and merge the changes.
@control-flow I apologize for the late response as I'm super busy on my end. This is a good catch... I see what you're saying as I'm creating a new instance for translations, and options every time the reducer runs.
It appears this one test is broken though, and can not confidently merge this until that is fixed. Have you tried pulling master, and running the tests locally to confirm they all pass? If the failing test is caused by this change I would need that fixed before this can be merged. I would offer my help, but unfortunately as I mentioned I'm crazy busy right now.
@ryandrewjohnson Sorry now I have been pretty busy on my side.
I ran all tests again locally on my fork this morning and it seems that all tests are now passing.
I don't understand what might have caused the error to disappear. I have no changes made to package.json / package-lock.json so the dependencies should be the same as back then when I ran the tests.
Is it possible to trigger the travis pipeline for this PR again and check if the error is still present in the travis build pipeline?
But just to clearify once again: I got the error shown above in your screenshot when I forked this repo and just ran the tests right after cloning. I had absolutely no changes to any of the tracked code files at all. So I assumed the tests are currently broken in master of this repo.
@control-flow there is definitely something strange going on with the tests. The master
branch has not been touched in 9 months and all I did was rerun that build from 9 months ago and it too now fails on that same test that is failing in this branch.
Like you if I pull latest from master and run the tests locally they pass. Unfortunately the only difference I can see between tests running local vs Travis is OS and unfortunately I don't have the time to dockerize this and test there.
Also as a quick fix I attempted to pull master and do a simple npm audit fix
to try and bring things more up to date in hopes that may fix this random test, but that ended up blowing up even more tests so i had to revert. Even now after the revert it's still showing that one random test failing.
Either way this will take some more digging into which unfortunately I don't have the time for right now. Will try to investigate when I get some down time.
As a workaround for this, we are using the following thin wrapper around localizeReducer
:
import { localizeReducer } from 'react-localize-redux'
const wrappedLocalizeReducer = (state, action) => {
const nextState = localizeReducer(state, action)
return (
state !== undefined &&
nextState.languages === state.languages &&
nextState.translations === state.translations &&
nextState.options === state.options
)
? state
: nextState
}