Feature discussion: createSlice behavior

[markerikson]

There's been some scattered discussion of how createSlice should actually behave, so I wanted to move this into its own thread.

I'll keep the initial thoughts short and open things up for discussion:

• I want createSlice to be included in some form. We're not removing it.
• At a minimum, I want to add the "listen for other actions" functionality from Add ability for slices to listen to other actions #83 / Alternate: Add ability for slices to listen to other actions #86 .
• We should make the selector behavior more useful
• We should consider the desired behavior around combining slices as part of a larger app's reducer.
• Given that createSlice was inspired by https://github.com/ericelliott/autodux , we should review what functionality autodux actually has for point of comparison. (Also lots of similar options in https://github.com/markerikson/redux-ecosystem-links, particularly under "Action / Reducer Generators".)
• Refer back to my "vision" statement in #82Discussion: Roadmap to 1.0 #82 (comment) for the purpose I want this to serve.

Tagging: @modernserf @denisw @Dudeonyx @BTMPL @lkuoch @mattkahl @doxick

[modernserf]

So I think I'm a little unclear on what you want selectors to do here. Right now it seems like a slice always has one selector, and it's in the form of { getCounter: (rootState) => rootState.counter }? Is there any reason for this particular API other than it matches autodux? When would you prefer counter.selectors.getCounter over something like counter.selector?

For "validation", i was imagining you'd create a wrapper for combineReducers that can accept slices and validate that they are in the state slot they claim to be in, like:

function combineReducers(reducersOrSlices) {
  const reducers = {}
  for (const [key, reducerOrSlice] of Object.entries(reducersOrSlices)) {
    if (reducerOrSlice.slice && reducerOrSlice.slice !== key) { throw new Error() }
    reducers[key] = reducerOrSlice.reducer || reducerOrSlice
  }
  return redux.combineReducers(reducers)
}

If you wanted slices to be nestable, you could have it such that giving a slice an id like foo.counter would respond to actions like foo.counter/increment and select (rootState) => rootState.foo.counter, with the path validations propagating through the tree of combineReducers calls.

[markerikson]

@modernserf : the primary reason for the current behavior is that that's what @neurosnap implemented for https://github.com/neurosnap/robodux , and our implementation is a literal port of what is in robodux to start with at the time I pulled it over. (As in, I cloned the robodux repo, changed the TS settings to spit out ES6+, compiled, and copied the JS files into RSK.)

As I said in the other thread, I think autodux actually iterates through all the keys if your state is an object and generates selectors for all of those. It looks like @Dudeonyx 's TS fork of RSK does something similar: https://github.com/Dudeonyx/redux-ts-starter-kit/blob/master/packages/slice/src/selector.ts .

[neurosnap]

@markerikson is correct. The intention was to eventually add more features to the selectors object returned from createSlice.

Overall I'm not happy with selectors returned by createSlice but don't really know how to improve upon it. For me it's really important to see how selectors work, because it is one of the more critical areas in terms of tweaking performance so I'm a little apprehensive on where to take it next. Returning all the keys within initialState as selectors seems fine, but the most important thing for me would be proper typing.

[Dudeonyx]

@neurosnap I eventually figured out the typing for generating additional selectors for all the keys of initialState when it's an object. I've been meaning to make the PR at the robodux repo.

[Dudeonyx]

Also I would like to propose that we change the default selector to have a fixed name instead of the current get${slice} for the following reasons

• It's potentially confusing when dealing with camelcased slices. For example it's slightly unintuitive to figure out the selector created for a slice named cData, will it be getCdata or getCData or getcData. This is a potential source of hard to determine errors.

• A fixed name like getSlice or selectSlice allows for IDE autocompletion and eliminates the previous issue.

• For typescript users a fixed name is type safe.

• The selector can easily be renamed with object destructuring. i.e

const { selectors: { getSlice: selectCounter } }  = createSlice({ 
  slice: "counter",
  //...
})

[markerikson]

@Dudeonyx : agreed. Also, I'd prefer to use selectFoo as the prefix rather than get.

[lkuoch]

Here's a suggestion I posted earlier but was closed in favor of this thread.

A possible improvement would be for slices to contain references to other slices.

I was thinking something like this:

• You only need to create a root slice which can reference every other slice

const rootSlice= createSlice({
    references: [slice1, slice2] // Maybe this or something similar
})

• Then you effectively have name spaces and only need to create one file with all your actions/reducers etc

// slice1.js
const sliceOne = createSlice({
    // initialState, reducers etc. 
    references: [slice3] // Refers to another slice
})

Then if there is an ergonomic way to access slices within slices you can effectively having implemented a namespace.

[Dudeonyx]

@Ikuoch How do you imagine the referenced slices being used?

[lkuoch]

@Dudeonyx In what context did you mean?

The way I was thinking is that that ideally for an app, you could create a rootSlice and register all your slices with it.

With slices, I think it eliminates the need to have your actions, reducers etc. split across files so that you can have them all in one data structure.

Then in your components, you only need to call the single source of truth, the rootSlice.

I haven't worked out how it would look like or work yet but maybe you can call something like rootSlice.references[slice1]

or...

there could be some magic where if you pass in a string to rootSlice.getSlice('slice3/slice5') it can interpret that as meaning you want to access rootSlice->slice3->slice5.

[denisw]

I want createSlice to be included in some form. We're not removing it.

Fair enough. I will then refrain from the createSlice rant I promised in #82. 😅 I'd like to raise two points, though, that we should be aware of when planning next steps:

• It promotes the idea that actions are somehow "owned" by reducers, making it less obvious that, of course, one slice's actions can also be reacted on by another slice. We can mitigate this with documentation.

• It veers somewhat into the territory of "Redux module", and keeps the line to that blurry. We need to make the line sharper to avoid confusion and limitless feature requests (more selectors, adding thunks and epics and sagas to slices, and so on). [*]

Given these points, I propose we create and communicate a razor-sharp definition for what a slice is or is not, and ensure that we don't grow its scope past that definition. Personally, I think we should stay with what we have currently, meaning that a slice is

• a reducer with
• a set of namespaced actions primarily handled by that reducer and
• a single selector for retrieving the whole slice state

I see no compelling reason to allow the definition of additional selectors as part of createSlice(). There are no gains in terms of reduced boilerplate, and if we accept that slices are not full-blown Redux modules, then having other selectors not directly attached to the slice object is no issue.

I also don't think it is a good idea to automatically create selectors for attributes of a slice state object. Like auto-generating getters in Java IDE's, it encourages the mindless direct exposition of all state pieces without considering what data is needed by the view layer, and in which form.

Based on the slice definition above, the following are changes we could do to the current function:

• Replace the slice.selectors object with a simple, statically named slice.select(state) function.
• Add an extraReducers option to allow a slice to handle another slice's actions, as always mentioned multiple times.
• Perhaps rename the reducers option to actions, to drive the point home that actions are being defined together with the case reducers to handle them. This also makes the difference to extraReducers clearer.

---

[*] As an aside, it should be also noted that slices interact somewhat awkwardly with the Redux "ducks"-style module pattern. As a module author, you have the choice of directly exporting a slice plus any non-slice entities like thunk action creators, making the user guess when to write:

import module from './module'

module.actions.someAction()

and when they need to write instead:

import { someThunkAction } from './module'

someThunkAction()

Or you can hide the slice from the user by making all slice pieces top-level exports, like this:

const slice = createSlice({
  // ...
})

export const select = slice.selectors.getSlice
export const someAction = slice.actions.someAction
// ...

export default slice.reducer

but then you lose the conciseness you used createSlice for in the first place.

This dilemma is not a problem as such - maybe createSlice is just not a good fit for the ducks pattern, and could be called out as such in the documentation.

[denisw]

Another aspect we should consider is composability. As currently defined, slices are not composable at all. Most obviously, the generated slice selector has a hardcoded assumption on where the slice state is placed - namely, under an attribute named after the slice in the root state object. Install the slice reducer anywhere else, and the slice selector breaks.

More subtly, composing slices into bigger slices is not really possible because their means of composition "above" and "below" are asymmetrical. A slice reducer assumes to own a state object attribute (the one defined by the slice name), whereas the sub-reducers of a slice own an action type instead. This is a fundamental mismatch that is hard, or perhaps even impossible, to overcome.

But there are more problems. Try to imagine a combineSlices(...slices) that itself returns a slice:

const counterSlice = createSlice({
  name: 'counter',
  initialState: 0,
  reducers: {
    add: (state, { payload }) => state + payload
  }
})

const listSlice = createSlice({
  name: 'list',
  initialState: [],
  reducers: {
    add: (state, { payload }) => state.concat([payload])
  }
})

const slice = combineSlices(counterSlice, listSlice) 

How would slice look? What would slice.actions.add be - the add action creator of count, or that of list? To remove ambiguity, we could add them as slice.actions['counter/add'] and slice.actions['list/add'], but that would be awkward; or add them as slice.actions.counter.add and slice.actions.list.add, but then the shape is not that of a slice anymore. And what to do if both slices are named "counter"? Alternatively, we could chose to not add the sub-slices' action creators to the composed slice at all, which however would make it less useful. So, it's complicated. And I didn't even get into what to do with the slice reducers...

Now, there are two possible courses of action. One is to rethink slices so that they become composable, which boils down to putting fewer assumptions into them about how they are embedded (e.g., by removing the auto-generated slice selector). The other is to acknowledge that slices offer no or only limited composability - but still offer a lot of convenience for the cases they were designed for - and clearly document the limitations.

[BTMPL]

@lkuoch

With slices, I think it eliminates the need to have your actions, reducers etc. split across files so that you can have them all in one data structure.
Then in your components, you only need to call the single source of truth, the rootSlice.

Not sure I understand, but it sounds like you want to just import one root slice and be able to reference all other slices through it, vs. importing specific slices the components (containers) want to work with.

If so, I don't really like the idea - it feels too "magical" for me, and probably typing it in TS would be a mess again.

[neurosnap]

Let me describe how I have been using slices with success in my projects. There are a couple of key ideas that I think are best practices for using redux that avoids some of the arguments against slices.

1. Flat is better than nested. If the redux state is normalized, there is rarely a need for reducers to compose one another (or slices) beyond one combineReducer for the main redux object.

2. Reducers must be simple and only handle their one domain. If a reducer messages needs to be wiped on LOGOUT, that should happen within a logout side-effect. Having reducers listen to actions that are not associated with it leads to spaghetti code. Actions end up having unintended side-effects and the only way to truly understand what an action does is to grep for it in all of its reducers. Instead, keep all state updates colocated in one location that uses the actions that are already created for that reducer. If a lot of reducers need to be updated based on one action, use redux-batched-actions.

3. This is not a ducks replacement, this is a building block for creating modules or packages. Slices are not a 1:1 with modules or ducks, some of my packages have many slices.

// packages/messages/index.js

const messages = createSlice({
  initialState: {},
  reducers: {
    add: (state, action) => { ...state, action.payload },
  },
  slice: 'messages',
});

const messageIdSelected = createSlice({
  initialState: '',
  reducers: {
    set: (state, action) => action.payload,
  },
  slice: 'messageIdSelected',
});

const actions = {
  ...messages.actions,
  ...messageIdSelected.actions
};

const reducers = {
  [messages.slice]: messages.reducer,
  [messageIdSelected.slice]: messageIdSelected.reducer,
};

const getMessageByIdSelected = (state) => {
  const msgs = messages.selector(state);
  const messageId = messageIdSelected.selector(state);
  return msgs[messageId];
}

const selectors = {
  getMessages: messages.selector,
  getMessageIdSelected: messageIdSelected.selector,
  getMessageByIdSelected,
};

function fetchMessages() {
  return (dispatch) => {
    fetch('/messages')
      .then((resp) => resp.json)
      .then((body) => {
        dispatch(actions.addMessages(body));
        dispatch(actions.setMessageIdSelected(body[0].id));
      })
  }
}

const effects = {
  fetchMessages,
}

export { actions, reducers, selectors, effects };

This is how I create packages and I think it works really well. I'm able to have multiple slices, I can define whatever other selectors, actions I want. I can even create my side effect functions in this file.

4. I try to keep my side-effect functions separate from my actions. This makes it clear that this action isn't simply hitting the reducer. Whether it is an action that activates a saga or an action creator, I colocate them inside effects for a package.

It promotes the idea that actions are somehow "owned" by reducers, making it less obvious that, of course, one slice's actions can also be reacted on by another slice. We can mitigate this with documentation.

I actually like that actions are owned by reducers. When I create a slice no other action is allowed to interact with that reducer. This guarantees that other actions don't slip into the reducer. I think this could be a decision by design.

I also don't think it is a good idea to automatically create selectors for attributes of a slice state object.

I think I agree with you here. I currently do not even use selectors returned from createSlice. I would be in favor or removing it completely to avoid any confusion on what createSlice is supposed to do. And like others have said, we technically do not know what part of the state this slice is dealing with, we are assuming a flat structure ... which works based on my recommendations above but is not enforced.

In defense of createSlice, it has vastly reduced my "redux boilerplate" and because all of my actions/reducers are simple by design, I'm able to build on them even more with prebuilt slices like mapSlice and assignSlice like described in #92

I'm able to get the redux side of things created very quickly and spent most of my time in my side-effects.

Overall I think that createSlice is a building block for modules, something else needs to be built on top of it for a packages solution that includes side-effects

[denisw]

Reducers must be simple and only handle their one domain. If a reducer messages needs to be wiped on LOGOUT, that should happen within a logout side-effect.

With this approach, the logout side effect needs to have knowledge about all slices affected by the logout, which creates pretty tight coupling. In practice, this can mean that the logout module knows about pretty much every reducer in the system, which makes it an undesirably tight coupling point. I think we should rather encourage developers to embrace the publish-subscribe nature of Redux and let those slices react to the single logout action that need it; it allows you to introduce a new slice that reacts to logout with fewer changes to the existing Redux modules.

This is not a ducks replacement

I agree, and we should make this as clear as possible.

[neurosnap]

In practice, this can mean that the logout module knows about pretty much every reducer in the system, which makes it an undesirably tight coupling point.

It's either that or every other module needs to be dependent on logout. In terms of hierarchy I think it makes sense for logout to depend on all the other modules. I've worked on large projects that have done it both ways and I think logout importing every module was the most maintainable. I think in theory, letting reducers "subscribe" to any event is really powerful, but at least based on my experience, it has always led to confusing and less maintainable code. I want one place where I can see what happens when a user logs out. I don't want to have to check N number of places to see what's going on.

[markerikson]

I've seen arguments both ways ("multiple slices responding" vs "all the logic in one place").

FWIW, Dan has always said that "multiple slices responding" was the intended usage, and it's something I intend to promote more in the upcoming docs revamp.

Also, while you're always free to organize your reducers and actions however you want conceptually, I personally prefer to put as much logic into the reducers as possible.

Busy at work atm, but one other quick thought: as far as I can tell, createSlice does work as a "ducks" generator just fine. I whipped up a quick CodeSandbox as an example, but here's the important part:

import { createSlice } from "redux-starter-kit";

const slice = createSlice({
  initialState: 0,
  reducers: {
    increment: (state, action) => state + 1,
    decrement: (state, action) => state - 1,
  },
});

export const { increment, decrement } = slice.actions;
export default slice.reducer;

Default-exports a reducer, named-exports action creators. It, uh, quacks like a duck to me :)

[BTMPL]

It's either that or every other module needs to be dependent on logout.

And personally I would go with the other one. It's up to the reducer to know that it should support resetting itself in response to the logout action, something that the "extra reducers" mechanism would solve (or just merging it all into one reducers field)

[denisw]

@markerikson, on the "ducks" point: sure, I mentioned that approach in my earlier comment. You improved on it with the destructuring export (👍), but the point stands that you still have to repeat the actions, which weakens the boilerplate-saving benefit.

Anyway, this and the logout action discussion are moving off-topic a bit. Of the points I raised, I think others deserve more focus:

• Where do we put the line of what a sloce is or is not? Are multiple selectors in or out of scope? Should there be a selector at all? What about thunks and other things? etc.

• Should slices be made more composable (e.g., nestable)? If yes, how? If no, can we explain why we think we don‘t need the composability, and perhaps which practices we recommend instead?

[markerikson]

Swinging back around to this discussion. Thoughts:

• I see that robodux already removed the selector functionality in 4.0: https://github.com/neurosnap/robodux/blob/master/CHANGELOG.md#v400-03-13-2019
• Someone pointed out that trying to statically type a function with a dynamic name is hard
• Agreed that auto-generating selectors for every individual field reeks of Java getters
• That one selector is really vestigial atm anyway

So, at this point I'm set on removing it in the next minor release (yay pre-1.0 semver!).

Steering the discussion in another direction, I'm combing through the various issues and coming up with this list of other potential changes:

• PR WIP: createSlice() revision proposal #109 ("createSlice revision proposal"):
  • change the "slice" object to be the reducer function itself
  • change the slice parameter to be name
  • remove the "slice selector" entirely
• Issue Suggestion, possible issues and improvements #41 ("suggestions and improvements"):
  • expose the slice name as part of the output, such as a key on the returned slice object
  • make the slice parameter required
  • create actions that are tied to a slice by prefix, but not directly handled by a reducer
• Issue Create Async Action #76 ("async actions"):
  • add some kind of "effects" field as a place to write thunks (and possibly also a future "action listener callbacks" thing, for responding to dispatched actions)?
  • add some kind of "create async action" API that creates "start/success/failure" action triples and/or generates thunks that take a promise-returning function and dispatches those action types

I am already planning to remove the slice selector. I am mostly good with changing slice to name and making it required, and with making the "slice" object be the reducer function itself.

Any further thoughts or suggestions on these other items, especially the async/effect related stuff?

[RichiCoder1]

add some kind of "create async action" API that creates "start/success/failure" action triples and/or generates thunks that take a promise-returning function and dispatches those action types

An interesting prior art here that I've enjoyed using is redux promise middleware.

So something like this: (with some of the #109 ideas included)

const todos = createSlice({
    name: 'todos'
    initialState: [],
    reducers: {
       // this probably isn't actually correct but 🤷‍♂️
        setTodos: (_, { payload }) => payload,
        addTodo: (state, { payload: newTodo }) => state.push(newTodo),
        markCompleted: (state, { payload: index }) => state[index].completed = true,
    },
    effects: {
       // Dispatch and actions would probably be, from what I've seen of other state frameworks like Vuex, basically impossible to Type.
        syncTodos: async (state, action, { dispatch, actions }) => {
           const newTodos = await someSyncFunc(state, action.payload);
           dispatch(actions.setTodos(newTodos));
        }
    },
});

const ui = createSlice({
    name: 'ui',
    state: {
         loadingTodos: true,
         error: null
     },
     extraReducers: {
         // "todos/syncTodos/completed"
         [todos.actions.syncTodos.completed]: (state, { payload })=> {
             state.loadingTodos = false;
             if (payload.error) {
                 state.error = payload.error;
             }
         }
     }
});

Contrived, but does the general idea make sense?

[phryneas]

create actions that are tied to a slice by prefix, but not directly handled by a reducer

I don't know. For me, just using an empty function as a selector always worked pretty well. I don't think adding the complexity of another option would be worth to sometimes omit an empty function. (Although I get that some people are crazy about saving every last character).

add some kind of "create async action" API that creates "start/success/failure" action triples and/or generates thunks that take a promise-returning function and dispatches those action types

We already have the extended way of writing a reducer:

const testSlice = createSlice({
        slice: 'test',
        initialState: 0,
        reducers: {
          testReducer: {
            reducer,
            prepare: payload => ({ payload })
          }
        }
      })

What about extending that?

const testSlice = createSlice({
        slice: 'test',
        initialState: 0,
        reducers: {
          testReducer: {
            reducers: {
              start: /* ... */,
              success: /* ... */,
              failure: /* ... */,
            },
            prepare: payload => ({ payload })
          }
        }
      })

[markerikson]

Meh. I'm not sure how that's really any better than:

reducers: {
    testReducerStart() {}
    testReducerSuccess() {}
    testReducerFailure() {}
}

and just adds more complexity.

I'm inclined to punt on that idea for now.

[neurosnap]

As someone who has minimal influence on the overall API of redux-starter-kit, please forgive me while I inject my opinion again in this thread. I think that adding async actions to createSlice confuses the point of createSlice. To me, the purpose of createSlice is to automatically create actions and a reducer that are bound together to reduce the amount of boilerplate code required to set them up together.

To me, async actions can be just as easily created outside of createSlice without adding any boilerplate. By adding side-effects/async actions to createSlice we are slowly growing the scope of the function and it is morphing into a one-stop shop for building a feature.

Other apps already do this so one could argue we are slowly converging on an API:

• https://kea.js.org/
• https://github.com/rematch/rematch

However, I feel like if we want to go down that route then we should build on top of createSlice, not add to it.

I think the other suggestions for improvements make sense.

[markerikson]

Yeah, I've been glancing over at Rematch, Kea, and Easy-Peasy for points of comparison, and there's definitely similarities in how they handle declaring effects of some kind.

I'm not sold either way. I can see a point to having a place to declare thunks and listener callbacks. I can see a point to keeping createSlice minimal.

[markerikson]

I was this close to having createSlice return the reducer function itself, but eventually talked myself out of that idea (see #197 ).

That said, I do plan to include the generated reducers in the return object.

[jamiewinder]

Is there / should there be a way to read the root state inside a slice?

[markerikson]

@jamiewinder : no. See this Redux FAQ entry: https://redux.js.org/faq/reducers#how-do-i-share-state-between-two-reducers-do-i-have-to-use-combinereducers

[markerikson]

I think at this point the major design questions around createSlice have been resolved. We're not doing anything special with "combining slices", and the generated selectors are gone.

I'm still open to discussing the "side effects" aspect down the road, but that can be a separate point of discussion (per #76 , etc).