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

enforceActions still allows to modify observers out of action scope #1473

Closed
the-fedorr opened this issue Mar 30, 2018 · 9 comments
Closed

Comments

@the-fedorr
Copy link

the-fedorr commented Mar 30, 2018

When i configure configure({enforceActions: true});, i expect that it does what it have to: doesn't allow state modifications outside of actions.

But after creation observable (i use, for example, extendObservable) i can directly change the observable's values.

Digging the code source code, i found that function checkIfStateModificationsAreAllowed checks not only allowStateChanges (what is "true" if i'm inside of action), but also detect's the atom's observers length, and if there is no observers - it ignores configuration enforceActions and allowStateChanges.

In my project it causes that i can modify store whenever i want, until React component is being rendered. After that store is getting an observer (React component) and checkIfStateModificationsAreAllowed stops allowing me to modify the store. Looks strange, don't it?

I believe what you call "strict" mode should be really strict. Or, at least we could have an option to set "strict-strict" mode.

Also provide the source code i talk about:

function checkIfStateModificationsAreAllowed(atom) {
    var hasObservers$$1 = atom.observers.length > 0;
    // Should never be possible to change an observed observable from inside computed, see #798
    if (globalState.computationDepth > 0 && hasObservers$$1)
        fail$1(process.env.NODE_ENV !== "production" &&
            "Computed values are not allowed to cause side effects by changing observables that are already being observed. Tried to modify: " + atom.name);
    // Should not be possible to change observed state outside strict mode, except during initialization, see #563
    if (!globalState.allowStateChanges && hasObservers$$1)
        fail$1(process.env.NODE_ENV !== "production" &&
            (globalState.enforceActions
                ? "Since strict-mode is enabled, changing observed observable values outside actions is not allowed. Please wrap the code in an `action` if this change is intended. Tried to modify: "
                : "Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, the render function of a React component? Tried to modify: ") +
                atom.name);
}
@pavanpodila
Copy link
Member

I am noticing this as well. As suggested by @the-fedorr, this behavior should be guarded even when there are no observers. This will keep it more consistent.

@mweststrate
Copy link
Member

See https://github.com/mobxjs/mobx/blob/master/CHANGELOG.md#improved-strict-mode for some background of the current behavior

@kunlongxu
Copy link

kunlongxu commented Apr 11, 2018

@the-fedorr @mweststrate
I encountered the same problem, no matter where I put this code, it does not take effect
configure({ enforceActions: true });
How I can do?

@mweststrate
Copy link
Member

@linmodev by default mobx won't complain about the lack of actions if the observables are not observed yet. In the next minor, it will also be possible to set enforceActions: "strict" to make it even throw in these cases (so this is more strict then true)

@mweststrate
Copy link
Member

Released enforceActions: "strict" as 4.2.0

@whs
Copy link

whs commented Apr 17, 2018

I'm running into #338 with the new strict mode enabled.

@mweststrate
Copy link
Member

mweststrate commented Apr 17, 2018 via email

@urugator
Copy link
Collaborator

urugator commented Apr 20, 2018

I suggest to change the available options to:
"never", "always", "observed"
(to remain BC "observed" <=> true, "always" <=> "strict", "never" <=> false)

@mweststrate
Copy link
Member

In MobX 4.4 / 5.1 the config values have changed to never, observed and always

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

No branches or pull requests

6 participants