Should I put my data changing code in functions or only in actions?

Question

I'm currently writing a bus timetable app that uses an object type to model a timetable "document".

interface Timetable {
  name: string
  stops: string[]
  services: string[][]
}

Along with the type I have a number of functions that I would typically write as methods on a class if I were to use mutation. I use Immer primarily so I don't have to write lots of spread syntax. For example,

const addStop = (timetable: Timetable, stopName: string): Timetable => {
  return produce(timetable, (newTimetable) => {
    newTimetable.stops.push(stopName)
  })
}

To manage the state I'm using Zustand and Immer, though I feel my problem would be the same if I was using Redux. In my store I have an array of Timetable objects and then actions that also use Immer to reassign the currently selected timetable object:

        updateTt: (tt, index) => {
          set((state) => {
            state.timetables[index] = tt
          })
        },
        updateThisTt: (timetable) => {
          set((s) => {
            if (s.selectedTtIdx === null) {
              throw new Error("no selected timetable")
            }
            s.timetables[s.selectedTtIdx] = timetable
          })
        },

I then call the data changing function inside my React component, and call the updating action:

const onAddStop = (name) => {
  updateThisTt(addStop(timetable, name))
}

This works, but I'm not sure I'm doing it right. I now have two layers of Immer calls, my components now have data-modifying functions called directly in their event handlers, and I don't really like how the "methods" look even if, in the grand scheme of things, that is a minor fault.

I've considered:

• Moving all the data modification into actions. This seems like it's going to be less maintainable and harder to understand, with a lot of duplication around indexing the store's array of objects.
• Creating an action for each data-modifying function; then, calling these actions from my event handlers. This seems like it's going to create a bit of duplication, even if just in terms of names.
• Turning my Timetable type into a class, rewriting the data-modifying functions as mutating methods, and setting [immerable] = true and letting Immer do all the work in my actions. I've done this, but I would rather stick to the immutable record pattern.

For what it's worth, documentation for Flux, Zustand or Immer tend to show the first option there, and only occasionally; no app is simple as as counter = counter + 1. With regards to apps out there in the wild using the Flux architecture, what is the best way to structure this?

Answer 1

(I'm not familiar with Zustand and Immer, but maybe I can help ...)

There are always different approaches, I am suggesting my favored one here.

Make a clear distinction between "dispatching" an action, and the actual "mutation" of the state. (Maybe add another level in between).

Specific "mutation" functions

I suggest to create specific "mutation" functions, instead of generic ones, i.e.:

• instead of updateThisTt: () => { ...,
• use functions like addStop: () => { ....

Create many mutation functions as needed, each serving a single purpose.

Build the new state inside the "mutation" functions

Conceptually, use your immer producers only inside the mutation functions.
(regarding the store I mean. Of course, you can still use immer for other purposes).

This seems to be recommended practice as well, according to this official example:

import { produce } from 'immer'

const useLushStore = create((set) => ({
  lush: { forest: { contains: { a: 'bear' } } },
  clearForest: () =>
    set(
      produce((state) => {  // <----- create the new state here
        state.lush.forest.contains = null
      })
    ),
}));

const clearForest = useLushStore((state) => state.clearForest);
clearForest();

Inside your component you can now just call the "mutator" function.

const onAddStop = (name) => {
  updateThisTt( timetable, name );
}

Building the new state

If building the new state becomes complicated, you can still extract some "builder" functions. But consider the next section "Large files and duplication" first.

E.g. your addStop function could as well be called inside the Zustand mutation:

updateThisTt: ( timetable: Timetable, stopName: string ) => {
    const newTimeTable = addStop( timetable, stopName );
    set( ( s ) => {
        if( s.selectedTtIdx === null ){
            throw new Error( "no selected timetable" )
        }
        s.timetables[ s.selectedTtIdx ] = newTimeTable;
    });
}

Large files and duplication

Of course code duplication should be avoided, but there are always trade-offs.

I wouldn't suggest a specific way to go, but note that code is usually more read than written. Sometimes I think it is worth writing a few letters more, e.g. something like state.timetables[ index ] multiple times, if it makes the purpose of the code a lot more obvious. You need to judge that yourself.

Anyway, I suggest to put your mutation function into a separate file that does nothing else, this way it probably looks way more understandable than you might think.

If you have a file that is large, but totally focuses only on modifying state, and is consistently structured, then it is pretty easy to read, even if you have to scroll a few pages.

E.g. if it looks like this:

// ...

//-- Add a "stop" to the time table 
addStop: ( timetable: Timetable, stopName: string ) => {

   // .. half a page of code ...
   
},

//-- Do some other specific state change
doSomethingElse: ( arg1 ) => {

   // .. probably one full page of code ...
   
},

//-- Do something else again ... 
doSomethingAgain: () => {

   // .. another half page of code ...
   
},

// ...

Also note that these mutator functions are completely independent (or are they ? I would hope that Zustand uses pure functions here, isn't it ?).

That means, if it becomes complicated, you might even split multiple mutation functions into multiple separate files inside a folder like /store/mutators/timeTable.js.

But you can do that pretty easy anytime later.

Building "payload" ("action callers")

You might come to a point where you feel the need to have another "level" between event handler and mutation function. I usually do have such a layer, but I don't have a good name for it. Let's call it "action caller" for now.

It's sometimes not easy to decide what belongs inside the "mutation function" and what inside the "action caller".

Anyway, you can "build some data" inside the "action caller", but you should not do any kind of state manipulation here, not even with using immer.

Transform state vs create a payload

That is a subtle distinction, and you probably shouldn't worry about this too much (and there are probably exceptions), but as an example:

You can use some parts of the old state inside the "action caller", like:

// OK:
const { language } = oldState;
const newItem = {  // <-- This is ok: build a new item, use some data for that.
    id: 2,
    language: language,
};
addNewItemMutator( newItem ):

But you should not map (or transform) an old state to a new state, like:

// NOT OK:
const { item } = oldState;
const newItem = {  // <-- This kind of update belongs inside the mutator function.   
    ...item,  
    updatedValue: 'new',
};
addNewItemMutator( newItem ):

Another example

Not making a specific suggestion here, just an example:

Passing many values to a mutator might become confusing, like:

const myComponent = ( props ) =>{
    const { name } = props;
    cosnt value = useValue();
    
    // ...
    
    const onAddNewItem: () => {
        const uniqueId = createUUID();
        addNewItemMutator( name, value, uniqueId, Date.now() );
    },

Inside the event handler you want to focus on what you actually want to do, like "add a new item", but not think about all the arguments. Also, you might need that new item for something else.

Ao alternatively you could write:

const callAddItemAction = function( name, value ){

    const newItem = {
        name:        name,
        value:       value,
        uniqueId:    createUUID(),
        dateCreated: Date.now(),
    };
    
    // sendSomeRequest( url, newItem ); // maybe some side effects
    
    addNewItemMutator( newItem );
}

const myComponent = ( props ) =>{
    const { name } = props;
    cosnt value = useValue();
    
    // ...
    
    const onAddNewItem: () => {
        callAddItemAction( name, value );
    },