Skip to content
This repository has been archived by the owner on Jul 8, 2021. It is now read-only.

Preventing modifications of the existing state #44

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

slawojstanislawski
Copy link

Many people use ngrx-store-freeze, which deep-freezes the state so that one didn't modify an existing state during development. If that's used, applying changes to the state object passed to the @Action-decorated function (e.g. state.loading = true) will result in TypeError: Cannot assign to read only property 'loading' of object '[object Object]'. I tested that with these changes the error doesn't occur, as the modification would then be applied to the new state instance.

- updating readme file to reflect the above
- removal of the unused 'take' operator
src/factory.ts Outdated
}
state = result;
store = result;
Copy link

Choose a reason for hiding this comment

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

Why state was renamed to store?

Copy link
Author

Choose a reason for hiding this comment

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

After building and testing, during PR preparation I made some changes and introduced wrong variable name by mistake. I corrected the variable name.

@amcdnl
Copy link
Owner

amcdnl commented Mar 6, 2018

I wanted it to do this so you don't have to spread every single time when you make a small modification.

@slawojstanislawski
Copy link
Author

Alright, but when I tried it with ngrx-store-freeze, which I'm using to ensure the state remains immutable, an error was thrown, and the state modification was small, I toggled a boolean. I like having store freezing as this 'safety net' but I also wanted to refactor my app to use ngrx-actions, and with the current implementation it's one or the other.

@keitoaino
Copy link

Any progress on this PR? :)

@slawojstanislawski
Copy link
Author

I would prefer to use the original version instead of publishing a forked repo to npm. If there's no intention for merging it, could you please close the issue, as I'm a bit stuck in indecision as how to proceed :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants