mobx 6 not throwing an error (only warning) if using enforceActions, is this by design?

[johot]

It seems mobx 6 will only do a console.warn when using enforceActions. What is the reasoning for this? I loved the fact that it forced me to do things the "correct way" in previous mobx versions :)

If this is intended could an option be added to force it to throw also?

Thank you for all your great work!

[mweststrate]

Yeah this is intended, it is not hindering anything from a technical perspective, and yet another config option seems like overkill. Most other warnings don't throw either, so this is a bit more consistent and less obtrusive, especially since this has become the default, so there is no longer an opt-in.

…

On Thu, Jul 30, 2020 at 8:22 PM Johan Otterud ***@***.***> wrote: It seems mobx 6 will only do a console.warn when using enforceActions. What is the reasoning for this? I loved the fact that it forced me to do things the "correct way" :) If this is intended could an option be added to force it to throw also? Thank you for all your great work! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2417>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBFQQJM4VNGIVHKZDULR6HB7DANCNFSM4POO5EDA> .

[nixaboo]

I would rather it throws, it makes more sense to me-> especially since it actually performs the state change.

For those intrested in a dirty-hack:

let warn = console.warn;
console.warn = (t) => {
if (t.indexOf('[MobX] Since strict-mode is enabled') >= 0) {
throw t;
}

warn(t);

};

It's not ideal but better than forking MobX :)

[mweststrate]

tip: use patch-package instead

…

On Mon, Dec 20, 2021 at 3:00 PM nixaboo ***@***.***> wrote: I would rather it throws, it makes more sense to me-> especially since it actually performs the state change. For those intrested in a dirty-hack: let warn = console.warn; console.warn = (t) => { if (t.indexOf('[MobX] Since strict-mode is enabled') >= 0) { throw t; } warn(t); }; It's not ideal but better than forking MobX :) — Reply to this email directly, view it on GitHub <#2417 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBH2ADZSET2GRNFCJXLUR4ZGTANCNFSM4POO5EDA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[nixaboo]

Niiice! Didn't know this existed, very useful!

FYI for those doing it as well, edit the files in the dist folder of mobx

[upsuper]

Hey @mweststrate,

In Canva, we find it hard to enforce this rule across a massive codebase and large number of developers in the organization without having it throw an error. Very few developers and basically no reviewers would notice an extra warning a change introduces if everything else works as expected. So we also want it back to throw an error instead of just issuing a console warning.

I would also argue that if it doesn't interrupt the normal flow, it is not really "enforcing" anything, just warning, so the name of the option is very misleading.

I can see in f60c913, you changed its behavior because you want to make it default without breaking too much existing code that doesn't enforce it currently. However, such change in behavior breaks our reliance on it to actually enforce the rule.

I suggest that a new option should be introduced for the purpose, maybe something like requiresActions to be consistent with other options that warn rather than throw. Alternatively, maybe consider making enforceActions accept more options like { on: 'always' | 'observed', level: 'warn' | 'throw' }, and map the existing string values to be with level: 'warn'.

It would be really unfortunate if we have to patch an actively maintained package like MobX, especially given that we use it in multiple packages.

Could you reconsider the decision?