ERROR TypeError: Cannot assign to read only property 'auth' of object '[object Object]'

[whisher]

Hello there,
I've got this ugly error using rehydrate: true
with "@ngrx/store": "^7.0.0",

ERROR TypeError: Cannot assign to read only property 'auth' of object '[object Object]'

export function localStorageSyncReducer(
  reducer: ActionReducer<any>
): ActionReducer<any> {
  return localStorageSync({ keys: ['auth'], rehydrate: true })(reducer);
}

[bufke]

Is this only with version 7? ie it works fine for you previously?

[whisher]

Only for version 7 in version 6, it worked fine

[bufke]

@btroncone I might suggest reverting version 7. Seems like it breaks things and we don't have a handle on why that is. Unfortunately it "works for me".

[rafa-suagu]

@whisher you are using the https://github.com/brandonroberts/ngrx-store-freeze library in the same project?

[whisher]

@rafa-as Yes I've got ngrx-store-freeze in my project
If I get rid of ngrx-store-freeze the app works.
I've always used ngrx-store-freeze in my project along with this lib
with no problem so I'm wandering what's going on?

[victor-ca]

#112

[victor-ca]

hello, these particular lines are the culprit for this issue;

but to fix that i have some questions regarding behavior, @btroncone could you please help me out on these:

1. what should happen if we have an initial state and a hydrated state with type mismatch, for example

 let initial= { astring: 'test'};
 let hydratedState = { astring: [] };
// i expected: final = { astring: 'test'}
// actually: final = { astring: []} 

// quoting from tests:
//* localStorage starts out in a "bad" state. This could happen if our application state schema
//* changes. End users may have the old schema and a software update has the new schema.

2. what should happen if we have an initial state and a hydrated state with additional props? now they are merged, is this the expected behavior?; ex:

 let initial= { a: 'test'};
 let hydratedState = { b: [] };
// now: final = { a: 'test', b: []} 

the lines above are for simplicity, here are the tests i actually used:

 it('merge initial state and rehydrated state', () => {
        // localStorage starts out in a "bad" state. This could happen if our application state schema
        // changes. End users may have the old schema and a software update has the new schema.
        let hydratedState = {
            ...initialState.state,
            oldstring: 'foo'
        };
        delete hydratedState.astring;
        localStorage.setItem('state', JSON.stringify(hydratedState));

        // Set up reducers
        const reducer = (state = initialState, action) => state;
        const metaReducer = localStorageSync({ keys: ['state'], rehydrate: true });
        const action = { type: INIT_ACTION };

        // Resultant state should merge the oldstring state and our initual state
        const finalState = metaReducer(reducer)(initialState, action);
        expect(finalState.state.astring).toEqual(initialState.state.astring);
        expect(finalState.state.oldstring).toBe('foo');
    });

    it('regression #111: should not mutate original state', () => {
        const origObj = {};
        let s1 = {
            state: {
                aobj: origObj
            }
        };
        let hydratedState = {
            aobj: {}
        };

        localStorage.setItem('state', JSON.stringify(hydratedState));
        const reducer = (state = s1, action) => state;
        const metaReducer = localStorageSync({ keys: ['state'], rehydrate: true });
        const action = { type: INIT_ACTION };

        // Resultant state should ignore astring due to type mismatch
        const finalState = metaReducer(reducer)(s1, action);
        expect(s1.state.aobj).toBe(origObj);
    });

    it('merge initial state and rehydrated state only if types match', () => {
        // localStorage starts out in a "bad" state. This could happen if our application state schema
        // changes. End users may have the old schema and a software update has the new schema.
        let s1 = {
            state: {
                astring: 'test'
            }
        };
        let hydratedState = {
                astring: [] // some array
        };

        localStorage.setItem('state', JSON.stringify(hydratedState));

        // Set up reducers
        const reducer = (state = s1, action) => state;
        const metaReducer = localStorageSync({ keys: ['state'], rehydrate: true });
        const action = { type: INIT_ACTION };

        // Resultant state should ignore astring due to type mismatch
        const finalState = metaReducer(reducer)(s1, action);
        // expect(finalState.state.astring).toEqual(finalState.state.astring); << succeeds because initial state is mutated
        expect(finalState.state.astring).toEqual('test'); // fails
    });

[rafa-suagu]

@rafa-as Yes I've got ngrx-store-freeze in my project
I get rid of ngrx-store-freeze and the app works sorry for that :)
quite annoying though :(
You can close the issue sorry for that again

@whisher the error is not on the freeze side is on this library. Check this comment: #107 (comment)

[btroncone]

Is there agreement that it would be better to revert and cut a new release? If so I can have it out tonight.

[victor-ca]

Hello Brian, first of all thank you for the library, it saved us a lot of time.
I think it makes sense to revert it; a solution for everyone might require deep-merge;

in my project i'm using a combo of deep merge and custom serializer, and during deserialization i ditch the stored state if it doesn't match appVersion; this way one doesn't have to deal with the decision when a certain state slice should or shouldn't applied

if its acceptable i could pull something similar in this repo:

// global appVersion
interface IFooSerialized extends IFoo {
version:any;
}
export function serialize(state: IFoo): IFooSerialized {
    return {
        ...state,
        version: appVersion
    }
}
export function deserialize(storedState: IFooSerialized): IFoo {
    if (!storedState || storedState.version !== appVersion) {
        return onboardInitialState;
    }
    let state = { ...storedState };
    // avoid app state pollution
    delete state.version;
    return state
}

[btroncone]

@victor-ca Thanks, if there are no strong objections I’m going to revert tonight as a 7.0 release. We can explore options like above as part of a next release. Does this work for you @bufke?

[bufke]

Working on a fix here #113 up to you if we should just fix or revert it. It should be a rather simple fix but I'm getting that strange error with the unit test complicating it.