Enforce immutability for event payloads?

[DawidLoubser]

For a framework that enforces a data model composed of immutable data, would it not make sense to enforce immutability for the event payloads?

A common pattern, when updating a store, is to set or deeply merge the event payload into the Immutable.Map or Immutable.List representing the store state. It's error-prone to always have to manually convert the payload to an immutable object - I can see no use-case for ever dispatching a mutable event payload.

Proposal: In flux.dispatch(...), always perform Immutable.fromJS(...) for a non-null payload.

I'm happy to add this and send a pull request, this is just a request to see if anybody else feels this way?

[jordangarcia]

Hi Dawid,

I can shed some clarity on this decision to be unopinionated about action payloads being mutable or immutable.

At Optimizely every payload is mutable. One of the great things about flux is it allows any part of the system to declaritively say something has happened (an action) and not have to worry about implementing the changes that action has on the state of the system.

By enforcing that all action payloads are immutable then that means any callsite of flux.dispatch now has to include the ImmutableJS library. One possible use case for Nuclear is to have a third party script be able to communicate with a Nuclear system by calling dispatch. Or using a websocket to send actions across clients - enforcing an immutable payload here adds complexity.

always perform Immutable.fromJS(...) for a non-null payload.

This is problematic for certain use cases. At Optimizely we use a Nuclear Store to track API requests in progress and store holds Deferred objects. This is something that can be coerced to immutable and then back to JS.

I think in certain projects this constraint makes total sense, especially if you are using Immutable more prevalently throughout your frontend stack. If this is something that you'd like strictly enforce you can always subclass the Reactor in your own implementation and this functionality.

[DawidLoubser]

Thanks, Jordan.

I find it rather ironic that the very first blanket statement made in the project README:

All app state is in a singular immutable map, think Om.

Is something that you violate in your own projects. Storing a deferred object in a store which is supposed to contain only immutable, easy-to-reason-about-at-any-time state is something that sounds dirty to me. It sounds like the state of your stores are updated not only synchronously and ordered based on your action handlers, but "at any time" when deferred computations are resolved - doesn't this put you back into the classical shared-mutable-state MVC mess? What if multiple parts of your state are affected by the outcome of your deferred, which is in one store? (or do I mis-understand?)

Of course, this wonderful project is yours, and you are entitled to be un-opinionated. Your code and documentation just clearly have different opinions :-)

I'm perfectly happy to subclass the Reactor in my own app, which is indeed very opinionated about the immutable state thing (and so is Om!). Thanks for your reply.

[mindjuice]

I think you are misinterpreting Jordan's comment. Saying that event payloads are mutable does not imply that "All app state is in a singular immutable map" is false.

If you convert the payload in your action handlers with Immutable.fromJS(), as you have said you are doing, then everything in the map is still immutable.

I use Immutable.toJS() in all my store action handlers except for one. That one store holds a very large content library, which is a JSON object retrieved from the server. Converting this to Immutable takes a couple of seconds, and it's read-only in the app, so it seems a waste of time to convert it back and forth.

Perhaps what you want is for Immutable to always convert mutable arguments to immutable automatically. Wherever you do it though, there will be cases where the performance overhead is not acceptable, and that is reason enough for it to not be forced on everyone.

[jordangarcia]

Dawid, you bring up a few valid concerns, hopefully I can shed some light on them.

It sounds like the state of your stores are updated not only synchronously and ordered based on your action handlers

This is exactly the case! There is an implicit ordering based on store registration when an action is dispatched and every stores gets a chance to handle it. The difference here between this and other flux implementation is that the observers or listeners arent bound to the stores, but to the entire app state.

A problem arises around store action handling order when you have stores that compute values based on other stores. When a component subscribes to a store that depends on another store than depending on the order the component may get updated with stale information.

In Nuclear all change observers are handled after the entire app state has settled and there is a strict enforcement that no observe handler function can call a dispatch and mutate the state of the system, so every observer will get the same app state for each dispatch cycle.

"at any time" when deferred computations are resolved - doesn't this put you back into the classical shared-mutable-state MVC mess

I think this is a bit of an overstatement. 99% of our stores are storing immutable JSON-serializable data, its simply the 1% of uses cases (like storing a deferred) that leads to less than desireable, but necessary, mutable state.

To elaborate on our use case, we have a way to create a unique key per rest api call by hashing the request method, params and entity. If two places in system both try to request the same resource then it is redundant. By tracking reference to the Deferred object for that request we can now return that for the second caller and both pieces of the system will be using the exact same data when it comes back.

I'm perfectly happy to subclass the Reactor in my own app, which is indeed very opinionated about the immutable state thing (and so is Om!).

I love immutability, but I believe this library must be pragmatic. Om lives in clojurescript where everything is immutable by default. NuclearJS lives in javascript where we have certain edge cases that force us to store mutable values in our app state from time to time.

Thanks for the inquiries, its great to see how people interpret and use NuclearJS!

[DawidLoubser]

Thank you for the comments and clarifications, @mindjuice and @jordangarcia - good food for thought. I can see why you guys have edge-cases forcing you out of the "immutable everywhere" paradigm for performance reasons, etc.

I actually would have liked for Immutable to force immutability. Otherwise, it breaks the very definition of immutability - you can't claim to have an immutable map if, by default, all contained values are mutable unless you remember to manually make them immutable! But I digress, and accept this to be a quirk of ECMAScript, and the Immutable.js implementation.

Perhaps Haskell has spoilt me :-)

[mindjuice]

Indeed, I would like that for Immutable too if it were performant. What I would really like though is to not have to use that awkward Immutable API. I wish I could just use what appear to be normal mutative operations and have the system do the work behind the scenes of making copies, etc.

Maybe I need my own compile-to-JS language.

On a related note, I'm just learning Haskell (and Elixir).

[jtremback]

@mindjuice https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy

[mindjuice]

Thanks @jtremback. I read a bit about that recently here too: https://github.com/facebook/immutable-js/issues/21 The lack of polyfill will keep it a dream for a while though. I guess it wouldn't hurt to have a library ready to go though.

[jtremback]

A few questions:

@jordangarcia Is there ever a reason for developers to tweak the order of store registration?

@DawidLoubser Is your complaint really about Immutable.js itself?

@mindjuice Do you think the slowdown is because you are converting from JS? Is there some way to serialize it directly into Immutable.js?

[DawidLoubser]

@jtremback Not at all. Immutable.js is as lovely as it can be given the language and practical constraints. It wasn't a complaint either, just an observation that, if nuclear-js was really serious about enforcing an immutable data model, it could make life easier by automatically ensuring that event data (which is ultimately often set into, or merged with, the immutable model) is immutable also.

I totally understand that nuclear-js doesn't want to enforce the immutability to allow certain fringe cases or hacks - I just like it when, if a project is opinionated about a certain aspect, this is enforced - even taken for granted in the actual codebase.

I've been bitten in my project where I've forgotten to explicitly make event data immutable, and I couldn't for the life of me see why one would want mutable event payload data in the first place.

[jtremback]

Seems like it would be very easy to extend the .dispatch method with .toImmutable

[jordangarcia]

@jtremback

Is there ever a reason for developers to tweak the order of store registration?

We have never had to think about store registration order. Since the reactor is responsible for notify Getter observers then it will wait until all stores have handled the action (which is easy since store handlers are synchronous) then all observers get access to the same app state. Also there is a strict enforcement that no observer can mutate the state of the system (dispatches are strictly forbidden).

[mindjuice]

@jtremback It's definitely converting that was the problem. The data comes from a server, and it's not going to ever send it in any other format except normal JSON. I don't mind it being mutable that much. I have no need to ever mutate that data, so it's very unlikely to cause me a problem, even accidentally.