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

Cannot test reducer: Cannot assign to read only property #424

Closed
jon301 opened this issue Mar 11, 2020 · 13 comments
Closed

Cannot test reducer: Cannot assign to read only property #424

jon301 opened this issue Mar 11, 2020 · 13 comments

Comments

@jon301
Copy link

jon301 commented Mar 11, 2020

Hi

Given the following reducer:

export const alphaReducer = createReducer(initialState, builder =>
  builder
    .addCase(actions.addData, (state, action) => {
      const text = action.payload;
      state.data.push(text);
    })
    .addCase(actions.clearData, state => {
      state.data = [];
    })
);

When I try to test it

describe('alpha reducer', () => {
  it('should return initial state', () => {
    const state = alphaReducer(undefined, { type: '' });
    expect(state).toEqual({
      data: ['alpha initial text array'],
      user: {
        age: 12,
        name: 'hello'
      }
    });
  });
});

Then I get the following error TypeError: Cannot assign to read only property 'data' of object '#<Object>'

  ● alpha reducer › should return initial state

    TypeError: Cannot assign to read only property 'data' of object '#<Object>'

      4 |   it('should return initial state', () => {
      5 |     const state = alphaReducer(undefined, { type: '' });
    > 6 |     expect(state).toEqual({
        |                   ^
      7 |       data: ['alpha initial text array'],
      8 |       user: {
      9 |         age: 12,

      at Object.<anonymous> (store/alpha/alpha.reducer.spec.ts:6:19)
@jon301
Copy link
Author

jon301 commented Mar 11, 2020

I've found the solution. It was related to immer auto freeze feature

In your jest.config.js file, add the following line:

module.exports = {
  ...
  setupFilesAfterEnv: ['./jest.setup.ts']
};

Create a new file jest.setup.ts:

import { setAutoFreeze } from 'immer';

setAutoFreeze(false);

Then you're good to go

@phryneas
Copy link
Member

The question is why autofreeze would cause any problems here. expect(...).toEqual should not try to modify the state in any way.

@phryneas
Copy link
Member

I suggest you create a small runnable reproduction for the problem and take it up with either immer or jest - this seems like a problem that should be resolved.
Nothing we can do about it though, so I'm going to close this. But if you find something, please report back - I'm quite interested in that.

@phryneas
Copy link
Member

Found it. Looks like you were running into this jest bug: jestjs/jest#9531

@catamphetamine
Copy link

catamphetamine commented Apr 26, 2021

@jon301 thx, your solution worked for our app.
We're not running any "jest" thing, but it still makes the weird Cannot assign to read only property error message go away when modifying objects from Redux state directly.

import { configureStore } from '@reduxjs/toolkit';
import { setAutoFreeze } from 'immer';

// Fixes "Cannot assign to read only property" error message
// when modifying objects from Redux state directly.
// https://github.com/reduxjs/redux-toolkit/issues/424
setAutoFreeze(false);

...

@phryneas
Copy link
Member

@catamphetamine you should never modify any objects from the Redux state directly though. What are you trying to do there?

@catamphetamine
Copy link

@phryneas In our case, we can and we do. We have implemented "optimistic" updates like when you perform a server-side action on an object and you set the new .status on it without the need to do a full server-side refresh of the page section.

@phryneas
Copy link
Member

phryneas commented Apr 26, 2021

@catamphetamine you have to do that through an action & reducer nonetheless. Blindly editing the redux state from the outside is 100% forbidden in every case. It will cause bugs like your UI not updating.

@catamphetamine
Copy link

you have to do that through an action & reducer nonetheless.

We prefer not to.

It will cause bugs like your UI not updating.

It won't.
We can handle ourselves, don't worry.

@phryneas
Copy link
Member

This is stupidly dangerous. If you want to do that, please stop using Redux and start using a library that supports that pattern.
The complete core of Redux operates on the assumption that Redux store is never mutated and all ecosystem tools (like the debugger), libraries (like react-redux) and middlewares also operate on that assumption.

@catamphetamine
Copy link

Fiiine, fine. Geez.

@markerikson
Copy link
Collaborator

@catamphetamine : I'm going to echo what @phryneas said. That is a very bad idea and you should never have been doing that.

The number one rule of Redux is DO NOT MUTATE STATE. This is so critical that we repeatedly emphasize the importance of immutability in our tutorials and throughout the docs. We also emphasize that it's never correct to modify state outside of reducers.

I'm not sure what gave you the impression that this was possibly a good idea.

@catamphetamine
Copy link

I'm not sure what gave you the impression that this was possibly a good idea.

The simplicity of the bare implementation?
Perhaps you do have some crazy complexity inside, whatever.
I've changed the code to call actions instead of mutating the objects directly.
Thx.

@reduxjs reduxjs locked as resolved and limited conversation to collaborators Apr 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants