Prefixing redux action types

[Mohammer5]

Ever since I've used redux, it felt cumbersome and in-transparent to create unique action types. Sometimes it's easy to come up with a namespace-prefixed action type, sometimes it's really tricky and will make the code less readable in terms of natural English.

I've started a discussion in one of the slack channels, but move the discussion here:

@Birkbjo commented:

I try to prefix the constant with a domain noun. Like USER_SET_NAME and the action types for each domain/component tends to be in the same file anyway. That way it's easy to know what the action relates to. That said I don't think the idea is bad. I've used a similar approach to automatically add SUCCESS/ERRROR for async actions.

And @erikarenhill added:

but I'd might argue that the reducerName should be uppercase too, or do you want to keep it camelcase to differentiate the reducername from the action? maybe just another separator for that. I've seen apps using for example [REDUCERNAME] Action this however would be something that I believe we should do consistently across all apps so a design decision for the team/project and not per app

I came to the conclusion to try the following:

const actionTypes = prefixValues('reducerName', {
    ACTION_TYPE: 'ACTION_TYPE',
};

which will result in:

{
    ACTION_TYPE: 'reducerName => ACTION_TYPE',
}

Other possible delimiters would be :: or @, I just found => to be the most readable so far.

If you have any objections or ideas how we could do this even better/easier, feel free to criticize/comment!

Issues that would be nice:

1. What is the defacto standard right now for dhis2 apps?
2. Should we introduce an official standard for creating new / refactoring existing action types?

UPDATE:

I confused a few names here while writing this idea/text.. I don't want to prefix action types with the names of the reducer! I want to prefix actions with their own namespace. So if there are user actions, I want to prefix user actions with user. This has nothing to do with reducers in general. Sorry for this, I think this caused some confusion. I don't want to couple actions and reducers, that should be avoided at all costs IMO

[Mohammer5]

@ismay added:

When going from app to app it's easiest when the approaches to these kinds of things are similar Personally I'd do what @Birk mentioned. I think the cognitive overhead of a function that does the naming is bigger than just coming up with a more specific name. Plus it gets more difficult to search for it in the codebase

Here's my response to that:

Plus it gets more difficult to search for it in the codebase

What is more difficult? I never search for the actual text representation of an action type. And the action type string, when inspected in e. g. redux-devtools can still be copied the same way as before if you want to do a recursive search through the codebase

I think the cognitive overhead of a function that does the naming is bigger than just coming up with a more specific name.

I don't really see the issue here? Why is this a bigger cognitive overhead? you don't have to think about the prefix as it's the reducers name and you don't have to think about the uniqueness which you would have to do otherwise. Maybe I'm just not thinking about some scenarios here?

[ismay]

I don't really see the issue here? Why is this a bigger cognitive overhead? you don't have to think about the prefix as it's the reducers name and you don't have to think about the uniqueness which you would have to do otherwise. Maybe I'm just not thinking about some scenarios here?

To explain a bit what I meant here. If I were new to a codebase and I saw that action types are being prefixed with the reducer name that'd be a bit of an uncommon pattern. So that would take a little extra effort to figure out why that's been done, etc. Whereas Brik's proposal to prefix the domain (or something else appropriate) sticks to what I'd expect, so that'd be simpler for me as a newcomer to the codebase.

That'd be my initial opinion. Personally I tend to prefer not abstracting unless I'm pretty sure that I won't change it anytime soon, that's sort of why I'd prefer this I think. Plus it seems like the more common pattern.

[ismay]

But it all depends on what we're doing in all our other apps, what would be the biggest net time saver, what everyone else prefers, etc, etc. So this is just my inital, naive response to your suggestion.

[Mohammer5]

@ismay I see. I understand that it might seem weird for newcomers. At the same time, once it's understood (which shouldn't be too hard), the whole time of app development should be fairly straight forward.

If I can prefix the action type with a simple noun, it's nice (and would be my preferred solution if it always was straight forward), but there are action types which either get very long or are hard to read (which to me increases the cognitive load), at least in my experience, maybe I need to work on my naming skills.

For example when having a reducer for organisation units and another one for organisation unit group sets (which is something I encountered in the reports app), there'd be action types like

• ORG_UNIT_LOAD_OPTIONS_START
• ORG_UNIT_LOAD_OPTIONS_SUCCESS
• ORG_UNIT_LOAD_OPTIONS_ERROR

Those are kind of ok, but what about the group sets which belong to the org units. I wouldn't want to prefix them with just GROUP_SETS as that doesn't indicate that they're part of the organisation units stuff, and ORG_UNIT_GROUP_SETS_... is quit long and can easily be confused with the ORG_UNIT... action types.

Maybe there's a proper prefix for this, but just the fact that I have to think about this is more cognitive overhead to me than the unconventional way of prefixing action types which only happens exactly one time.

[Mohammer5]

In the end, the prefix is more or less the reducer's name IMO

[ismay]

One thing I didn't think about initially, but wouldn't this naming pattern tie actions to reducers, where you might want to respond to the same action in multiple reducers? That seems a bit confusing. Or am I misunderstanding your intention?

[varl]

I'm not a fan of a tight coupling between Actions and Reducers, and there is nothing in Redux which suggests that they should be tightly coupled:

Is there always a one-to-one mapping between reducers and actions?

No. We suggest you write independent small reducer functions that are each responsible for updates to a specific slice of state. We call this pattern “reducer composition”. A given action could be handled by all, some, or none of them. This keeps components decoupled from the actual data changes, as one action may affect different parts of the state tree, and there is no need for the component to be aware of this. Some users do choose to bind them more tightly together, such as the “ducks” file structure, but there is definitely no one-to-one mapping by default, and you should break out of such a paradigm any time you feel you want to handle an action in many reducers.

[1] https://redux.js.org/faq/actions#is-there-always-a-one-to-one-mapping-between-reducers-and-actions

[JoakimSM]

varl and ismay beat me to it. But I urge you to stay away from using a reducer name in the action type name. For the reasons varl mentioned.

[Mohammer5]

I absolutely agree to this and that's something I don't want either and is very important to me as well, so I'll explain a bit more in depth what I mean:

I keep the redux state relatively flat, so let's say there's a user state, it'll be top level on the redux state. This way, state is reusable and derived state is highly composable.

I also never add a trailing Reducer to the name of a reducer, a user state would have a reducer function with the name user, and actions file called user and all the actions will do nothing but express their intent in regards to the user.

Normally there will be reducers for handling state of a section/page displayed, which is more or less UI state and not state for values being stored in the database. Those states are not reusable as they're tightly coupled to the sections (if they're not tightly coupled to the section, then I'm not talking about those reducers in this paragraph). The name of those reducers won't have the name Reducer in their names either, but as their actions are tightly coupled to the section, it's ok to prefix their names with the section name, I'd say.

This doesn't mean that other reducers can't use other actions, their prefix will be something like a namespace anyways.

I'll give an example of what I've done in the reports app so far:

The reducers that are not for UI state of specific sections are:

• dataSet
• dataSetDimensions
• feedback
• htmlReport
• organisationUnits
• orgUnitGroupSets
• pagination
• period

And the UI section specific reducers are:

• dataSetReport
• reportingRateSummary
• resource
• standardReport

All action types that exist for the first list of reducers could be prefixed with the name of the reducer as their only intent is to do something within that domain.

So all the reducers can still work with any incoming actions, they are not coupled to the reducer, and the prefix still expresses the intent's domain

[Mohammer5]

this also means that when creating actions and prefixing them with the file's name, there doesn't need to be a reducer for those actions explicitly. And reducers can work with any set of actions regardless of their namespace

[ismay]

Normally there will be reducers for handling state of a section/page displayed, which is more or less UI state and not state for values being stored in the database. Those states are not reusable as they're tightly coupled to the sections (if they're not tightly coupled to the section, then I'm not talking about those reducers in this paragraph). The name of those reducers won't have the name Reducer in their names either, but as their actions are tightly coupled to the section, it's ok to prefix their names with the section name, I'd say.

Maybe local state would be a good place for this? Unless there's a lot of sharing, just using local state might help. I'm not sure, haven't taken a look at what's in your store. But keeping derived state and things that can be put in local state out of your store might also simplify it.

[Mohammer5]

Maybe local state would be a good place for this

I disagree. These states can (in the reports app they do) change on location change. With a reducer this is very easy as you just have to listen to the location change action.

Other than that I think it's very good practice to keep all state at one place only so you never end up with the problem of not knowing where to look for or place new state. If there's ever a new feature/requirement that needs the "local" state and the redux state to "interact", it'd be more complicated.

Derived state should be done with selectors (reselect is a nice tool here in my experience) IMO. Otherwise you might run into the scenario where you have to make sure reducers are executed in a certain order (Flux hat the waitFor functionality for this, and that was nasty...)

[amcgee]

I think I'm in favor of explicitly-prefixed actions. I think there's a possible way to improve action namespacing, but I don't think divergence between the variable name and the action string (without a direct constant mapping in code) is the way to go.

+1 to selectors and reselect

[Mohammer5]

Ok, I just realized what I wrote doesn't make sense! I don't want to prefix the action types with reducer names! I want to prefix action types with their namespace, regardless of any possible reducers reacting to those actions!

Sorry for that, will update the actual issue description! Update: See the updated issue description please.

[Mohammer5]

I think I'm in favor of explicitly-prefixed actions. I think there's a possible way to improve action namespacing

If actions were grouped by their namespace, then the explicit prefix would be same for all action types, right?

but I don't think divergence between the variable name and the action string (without a direct constant mapping in code) is the way to go.

The mapping would still be there (just transformed by the prefixing function). The only "drawback" is that the string representation doesn't match the constant's name when inspecting them in the redux-devtools. What are your concerns if they don't match 100%?

[amcgee]

If actions were grouped by their namespace, then the explicit prefix would be same for all action types, right?

Yes, true, but I meant explicit prefixes in the action variable + name like "NAMESPACE_GROUP_VERB='NAMESPACE_GROUP_VERB'", so the namespace is used everywhere the action is referenced.

What are your concerns if they don't match 100%?

The previously-mentioned inability to search the codebase for an action name is troubling to me - when debugging in a large, and particularly a large and unfamiliar, codebase this can be a barrier. It also should yield a clear mapping from the action name to every use of that action, rather than having 5 different actions in different namespaces all called "LOAD_START", just with different dynamically-generated prefixes.

Also, constant strings as action names allows us to do static code analysis in the future, we lose the ability to do that easily if we move to runtime-generated action names.

[ismay]

But keeping derived state and things that can be put in local state out of your store might also simplify it.

Ah I should have worded this better. What I meant here "keeping things that can be put in local state out of your store might simplify it". And "keeping derived state out of your store might simplify it". I didn't mean "put your derived state in local state". Selectors are also how I'd deal with derived state.

And as for local vs store. If it's shared between multiple components and it gets too convoluted passing it around, then yeah I'd also put it in the store. But otherwise local state is perfectly fine I think.

[varl]

And as for local vs store. If it's shared between multiple components and it gets too convoluted passing it around, then yeah I'd also put it in the store. But otherwise local state is perfectly fine I think.

Yeah, one of the principles of the React workflow is to start with local state, and as you realise that you need the state elsewhere you pull it up. Eventually you have nowhere left to pull the state and that's where Redux comes in.

[Mohammer5]

@amcgee I have to disagree with most of this:

so the namespace is used everywhere the action is referenced.

Action types are used only in reducer, everywhere else (ideally only in mapDispatchToProps), the action creators are used. Only in the reducer files, you'll import the action types. Either the reducer matches the namespace, then it's obvious, otherwise when using action types from different namespaces, you'll have to rename them in your imports anyways:

import { actionTypes as userActionTypes } from '../actions/user'
import { actionTypes as orgUnitActionTypes } from '../actions/orgUnits'

The namespace is always given when reading through code, even if it's missing in the action type name itself.

inability to search the codebase for an action name

You never have to search your code base for an action name, unless you look at the redux devtools output, which would state exactly where to look for it. In any other case, you'd be looking for the action creator, which is either very easy to find from the context (I'd say like 95% of the time) or you just have to look at the imports in the mapDispatchToProps function, which should be very easy to find as well. If that's not the case, it's not the fault of the action creators / action types but of the application architecture, which would be a larger problem...

Regarding the static code analysis, this is something I never worked with so far except for linting / formatting. Can you explain what static code analysis might want to do with the code here? I. e. what it wants to check the code for? Is there anything that we're doing already that I can have a look at?

@ismay

But otherwise local state is perfectly fine I think.

I really don't think so.. don't want to sound too harsh here.. I think separating state is bad practice, it's makes the codebase less deterministic and doesn't add any value. (especially when a new feature cause new derived state from both state sources). Either you handle state change a different way in the react component's local state and then you have two ways of state handling, which makes the app more complicated (which is bad) or you do it the same way at different locations, which is an anti pattern.

@varl

one of the principles of the React workflow is to start with local state

I don't think that it's a good workflow though.. ideally the view is 1. independent of the state and just represents the state visually and 2. it makes replacing the view technology much harder if there's something better than react at some point (and our apps will live for a very, very long time). But this also relates to what I mentioned above, it doesn't follow the SRP/SoC principle. I had to refactor so many components already because they had state and a new feature collided with that architecture.

[varl]

@Mohammer5 so just to be clear the idea is to choose between:

Standard

ORG_UNIT_LOAD_OPTIONS_START="ORG_UNIT_LOAD_OPTIONS_START"
ORG_UNIT_LOAD_OPTIONS_SUCCESS="ORG_UNIT_LOAD_OPTIONS_SUCCESS"
ORG_UNIT_LOAD_OPTIONS_ERROR="ORG_UNIT_LOAD_OPTIONS_ERROR"

Prefixed

ORG_UNIT_LOAD_OPTIONS_START="reducerName => ORG_UNIT_LOAD_OPTIONS_START"
ORG_UNIT_LOAD_OPTIONS_SUCCESS="reducerName => ORG_UNIT_LOAD_OPTIONS_SUCCESS"
ORG_UNIT_LOAD_OPTIONS_ERROR="reducerName => ORG_UNIT_LOAD_OPTIONS_ERROR"

Does this mean you want the reducerName in the prefixed variant to be the filename of the reducer?:

this also means that when creating actions and prefixing them with the file's name

I don't like this. That means that if the file is renamed, the actions will need to be updated too for the convention to match.

I'm leaning towards the standard way because it's simple and predictable.

[Mohammer5]

@varl First: Sorry for the confusion, I'm not talking about reducers at all. That was just my mistake at the beginning (please see the update at the bottom of the issue description)

Does this mean you want the reducerName in the prefixed variant to be the filename of the reducer?:

Let's say there's are user actions, then there most likely will be a file actions/user.js which groups all the user actions. And this is the prefix I'd want to use for all action types defined inside this file. If the prefix wouldn't suit, then the action type/creator is at the wrong location.

The standard way would be to prefix the name of the actions, so instead of SET_NAME, it'd be USER_SET_NAME. If you ever refactor this, if you change the file name, you most likely will have to change all the USER_, too. Effort-wise, there's no difference between a search and replace and renaming the value that's passed to the prefixValues function.

In case of the user actions, it'd look like this:

SET_NAME='user => SET_NAME'
DELETE='user => DELETE'

[ismay]

The short-hand is a bit confusing to me. With an action creator like this:

export const fetchUserSuccess = payload => ({
  type: types.FETCH_USER_SUCCESS,
  payload
})

What would you want to change exactly?

[Mohammer5]

@ismay the action creator code wouldn't change at all, I'd the the types definition. Let's say the fetchUserSuccess is in actions/user.js

Then I'd want something like this:

export const types = prefixValues('user', {
    FETCH_USER_SUCCESS: 'FETCH_USER_SUCCESS'
}

If I wanted to substitute the the function call with the return value, it'd look like this:

export const types = {
    FETCH_USER_SUCCESS: 'user => FETCH_USER_SUCCESS',
}

Doesn't sound like a good idea with FETCH_USER_SUCCESS in mind only, but stuff like SET_TYPE or SET_NAME could be reused with ease for many different action name spaces

[ismay]

Allright, that clears it up a bit for me. But then instead of doing this:

export const types = prefixValues('user', {
    FETCH_USER_SUCCESS: 'FETCH_USER_SUCCESS'
}

Why would you not do:

export const FETCH_USER = 'FETCH_USER'
export const FETCH_USER_SUCCESS = 'FETCH_USER_SUCCESS'
export const FETCH_USER_FAIL = 'FETCH_USER_FAIL'

What's the added benefit? To me it's a bit unclear what problem the abstraction is meant to solve.

[varl]

But this also relates to what I mentioned above, it doesn't follow the SRP/SoC principle.

Separations of concerns depends on how you slice your concerns.

Given that user actions is a domain, I don't see the point of prefixing everything according to the domain either.

In the system of naming actions USER_LOAD_DATA the concern to separate out is the user actions.

If we name the action LOAD_USER_DATA, the concern is to load data, and we would try to separate out that concern.

Both are valid ways to separate concerns. The Reports App in its design seems to align more with the former, but that does not mean that all other applications have decided to slice their architecture the same way, so I don't think we should apply a rule to this before deciding on how we want to slice our concerns.

[Mohammer5]

@ismay because that makes it harder to import action types from different files. Some thunks dispatch actions of 3 different domains and you might have a few thunks inside one file.

it'll reduce the amount of work for importing action types (you can still spread-assign them to variable names in the files that you want to use them in. If you import everything from a module to get all the action types, you'll import everything else as well, which makes code harder to understand (because it's less declarative which of the imported values you're going to use in the code).

But in the end, if you prefix each action explicitly, it's not that much of a difference, just more import work with a bit less clarity.

@varl

so I don't think we should apply a rule to this before deciding on how we want to slice our concerns.

That's not my intention. It was more a general idea to think about this as well. I was more or less trying to figure out whether it's feasible to prefix action types in general or if there are any reason against it.

To me, the only heavy-weighing argument so far is the static code analysis, which I have no idea what the implications are.

Separations of concerns depends on how you slice your concerns.

When I mentioned SoC, it was regarding having state in react components, not the actions prefixing topic, sorry if that was unclear

[varl]

I don't think need a function to add a prefix which is only going to be typed once.

But if we look at the result instead:

export const types = {
   FETCH_USER_SUCCESS: 'user => FETCH_USER_SUCCESS',
}

If we import that types definition somewhere else, you would have to give it a name anyway, and usage reads nicely in the code.

import { types as UserTypes } from 'actions/user.js'

store.dispatch(UserTypes.FETCH_USER)

So to me it seems that the discussion is about what we want to see in the Redux developer tools?

[Mohammer5]

So to me it seems that the discussion is about what we want to see in the Redux developer tools?

Nope, where I come from is:

There are actions with the same intention on different domains. Users can have a name, so there will be a SET_NAME action for users. Reports can have a name, so there will be a SET_NAME for reports, same for resources. And it's not just the name, it can be many things. Many times I'm annoyed that I have to prefix the action type with all the domains, so I was wondering if I can "automate" this with a function that does that automatically while still using SET_NAME in the code.

The only difference would be the output in the redux dev tools and using static code analysis.

If we import that types definition somewhere else, you would have to give it a name anyway, and usage reads nicely in the code.

Exactly, that's what I replied to @amcgee as well, if you need to use the actions anywhere, they'd be namespaced anyways.

[ismay]

I think I understand what you mean now.

If you import everything from a module to get all the action types, you'll import everything else as well, which makes code harder to understand (because it's less declarative which of the imported values you're going to use in the code).

I don't think that that's a problem. I've personally never found that confusing. I assume that you mean that importing like so:

import { * as userActions } from 'actions/user.js';
import { * as dataActions } from 'actions/data.js';

// and then you have a couple of actions from other files/domains
store.dispatch(userActions.FETCH_USER);
store.dispatch(dataActions.SUBMIT_DATA);

Is unclear, because you're importing all the actions, and that might be confusing other devs. To me that's not a problem. I wouldn't find that confusing.

[Mohammer5]

If we import that types definition somewhere else, you would have to give it a name anyway, and usage reads nicely in the code.

What I forgot to mention here, if userAction.FETCH_USER represents the string FETCH_USER, then you couldn't have another FETCH_USER anywhere else (ok, this exact type is a stupid example, there should be no other fetch user action...), For all the switch cases, there'd be no difference between userActions.FETCH_USER and otherDomain.FETCH_USER if the action types were not prefixed

[Mohammer5]

@ismay Yes, I can do that. To me it means that userActions contains action types of users only, which is not the case and could (probably wouldn't, but I prefer to be careful with could/would) cause bugs/weird behavior. IMO a variable name should always express exactly what it contains, which is not the case here. You could give it another name, that might work.

Besides that, I'd have to prefix every single action type, which is exactly why I started this issue.

[ismay]

To me it means that userActions contains action types of users only, which is not the case

Hmm ok? So then the name's incorrect right?

[Mohammer5]

Yeah, just the name, if it was sth like userActionModule it would be correct.

[varl]

Ah, OK. Thanks for explaining it so it gets through my thick skull.

For all the switch cases, there'd be no difference between userActions.FETCH_USER and otherDomain.FETCH_USER if the action types were not prefixed

You're right. Symbols would be more appropriate in that pattern.

Many times I'm annoyed that I have to prefix the action type with all the domains, so I was wondering if I can "automate" this with a function that does that automatically while still using SET_NAME in the code.

Hm. OK. I think this will lead to a confusing code base because of the lack of consistency.

import { FETCH_DATA } from './actions/user.js'  // export const FETCH_DATA=...
import { types } from './actions/report.js'  // export const types = {}...
import { types as analyticsTypes } from './actions/analytics.js' // export const types = {}...

...
// many lines of code
...

store.dispatch(FETCH_DATA)
store.dispatch(types.FETCH_DATA)
store.dispatch(analyticsTypes.FETCH_DATA)

I much prefer the consistency of having this everywhere in the code base.

store.dispatch(FETCH_USER_DATA)
store.dispatch(FETCH_REPORT_DATA)
store.dispatch(FETCH_ANALYTICS_DATA)

Code is "write-once read-many", so we should optimise for readability over how convenient it is to write the code. To me the longer more explicit form wins hands-down in how easy it is to read.

[ismay]

Ah, so basically you want to avoid typing these:

ORG_UNIT_LOAD_OPTIONS_START="ORG_UNIT_LOAD_OPTIONS_START"
ORG_UNIT_LOAD_OPTIONS_SUCCESS="ORG_UNIT_LOAD_OPTIONS_SUCCESS"
ORG_UNIT_LOAD_OPTIONS_ERROR="ORG_UNIT_LOAD_OPTIONS_ERROR"

And prefix them automatically. Sorry, it's probably because it's friday that that took me a while 😋. I don't know, maybe dealing with it in your editor instead of in your codebase would be better then (like multiple cursors in atom, or a macro in vim)? Then you have the best of both worlds, automation + simple code.

[amcgee]

Yeah, one of the principles of the React workflow is to start with local state, and as you realise that you need the state elsewhere you pull it up. Eventually you have nowhere left to pull the state and that's where Redux comes in.

I agree with this, one anti-pattern I see a lot (not necessarily here) is automatically putting everything in Redux. While Redux is intended to be a single source of truth for application state that doesn't mean all state should be in Redux (the distinction being between business logic and user interaction logic). I think hooks will help make local state much easier to implement for interaction logic (dialog state, textbox state, error state, etc)

I much prefer the consistency of having this everywhere in the code base.

While @Mohammer5 made a fair point about the action constants often being wrapped in action creators, hence abstracting away the action name itself, that isn't guaranteed to always be the case for complex logic or use of the same action in multiple reducers. I agree with @varl that I'd prefer canonical and matching action type definitions.

I like to namespace things heirarchically with left-to-right increasing specificity, always ending with the verb, as this makes context explicit. So, to steal @varl's examples:

REPORT_DATA_FETCH instead of FETCH_REPORT_DATA
FILTER_ACTIVATE
REPORT_COLABORATOR_PERMISSIONS_ADD, etc.
<NOUN>(_<NOUN>)*_VERB generically

For action creators, the names are the "human readable action form"

fetchReportData
activateFilter
addReportColaboratorPermissions
<verb><Noun>

For selectors:

getReportData()
getActiveFilter()
or
getReportData() - this might be a reselect memoized selector which returns a subset of the report data after passing through the active filter.  Since it can be memoized it is only re-calculated when either of the upstream state elements change

With these naming schemes actions, action creators, and selectors are all exhaustively self-describing, so there's no ambiguity when you see one referenced and no need for dynamic prefixing. Just my 2c.

I think this mostly matches the philosophy in this Medium post, though I had not previously read it: https://medium.com/@kylpo/redux-best-practices-eef55a20cc72

Just my 2c

[janhenrikoverland]

One thing I would like to add: I don't think exporting an actionTypes object is a good idea. I think action type constants should be named-exported one by one so that you get an error if an action type constant is renamed and you didn't update it accordingly across the app. If you do actionTypes.NO_LONGER_VALID_ACTION_NAME it's just undefined and your reducer will not catch it.

[Mohammer5]

I had two nights to think about this. While I still think that prefixing action types makes sense inside the functionality frame, it's quite the contrary when it comes to working with it (which is basically what most of you mentioned rightfully).

I'll gather the reasons here:

1. The gains from having not having to prefix the action types manually are so little that it's not worth going against conventions here.
2. It'll work against consistency across multiple apps
3. It's confusing to those who encounter it the first time / have to think about doing this while creating new action types (I haven't thought about this one whatsoever when @amcgee mentioned the cognitive overhead)

So I'll return to prefixing both the name and the action type string with the domain, even if that means that the action type might get quite long. The action types won't be used anywhere but inside reducers and action creators, so one normally doesn't see them when working with the business logic.

@amcgee

I agree with this, one anti-pattern I see a lot (not necessarily here) is automatically putting everything in Redux.

I second that, but to me there's a subtle difference between what you wrote and what @varl quoted. Putting application state into components and move the state up until it eventually ends up on the redux level means that you'll have different places for app state. There are still valid examples to keep state inside components, but in my experience, these are interactive components meant to be reused (input or other form elements, or in our case the sharing dialog). I still think that application state should move 100% to redux' store.

@janhenrikoverland

you get an error if an action type constant is renamed and you didn't update it accordingly across the app.

This should be caught by both code reviews and unit tests (one of the reasons why I think the real actions creators should be used to create actions when testing a reducer). Besides that, in terms of readability I really like named action constants.

@ismay @janhenrikoverland

import { * as userActions } from 'actions/user.js';

I thought about this as well. Although the name userAction is not an accurate description of what's inside the variable, it probably won't be responsible for bugs. I changed my opinion on that and like it quite a bit now.. Probably will encounter both ways (storing action types with const or inside an actionTypes object) and will keep an eye on what seems to work better in terms of maintainability and readability!

@varl

store.dispatch(FETCH_DATA)
store.dispatch(types.FETCH_DATA)
store.dispatch(analyticsTypes.FETCH_DATA)

Something like this should never happen! It should be consistent through out the whole app, regardless of the approach, but I suppose we all agree on that.

[varl]

Putting application state into components and move the state up until it eventually ends up on the redux level means that you'll have different places for app state.

A workflow is a heuristic, it does not make any assumptions about what is application state and component state. Sometimes it is very clear which is which, yet sometimes, especially on new development it is not immediately clear.

Personally I like to partition complexity, and that means that most state starts out as component state (probably hard coded). As I iterate on the code I realize what parts of the state is under application control and which state is isolated to the component. What helps me realize where state goes is the "pull state up" workflow.

Awareness of when to stop is a requirement. The point is not to pull all the state up to the application state, the point is to pull it up as high as it needs to go. I find that this keeps everything as simple as it can be, and as far away from YAGNI as possible.

Something like this should never happen! It should be consistent through out the whole app, regardless of the approach, but I suppose we all agree on that.

Yeah, we agree on that. In my experience is that if it is open to be used in different ways, it is going to be used in different ways. It should never happen, though if it could happen, then it is probably that it will happen.

Which is why I think it's good that you brought this whole discussion up.

• The gains from having not having to prefix the action types manually are so little that it's not worth going against conventions here.

• It'll work against consistency across multiple apps

• It's confusing to those who encounter it the first time / have to think about doing this while creating new action types (I haven't thought about this one whatsoever when @amcgee mentioned the cognitive overhead

I think you summed it up neatly here. Mind if I close this discussion as settled, @Mohammer5?

[Mohammer5]

@varl Yup! I'll close it

[amcgee]

Agree with all these latest points @varl @Mohammer5. I do think it's important, though, not to bury application state within components (even if the idea is then to "bubble it up" to application state). I think a good test for this is that a redux state snapshot should be something you could use to reproduce a bug. If a user gave you a snapshot of their redux state you should be able to reproduce their environment (derive an application from the state snapshot) without knowing anything else about their environment. That could be taken to the extreme and everything be kept in application state, but to be maintainable in code it would require a sophisticated mechanism for state fragmentation like the theoretical below.

One quick point to add:

I thought about this a little more as well and think there is a hypothetical use case for dynamically-namespaced actions. I actually realized that I implemented this in another project a couple years ago. In the nominal and conventional use-case of a standalone application I maintain that static constants with explicit namespaces (by convention) are probably the way to go.

The case I think that could be made for dynamically-prefixed actions is this (and it's a weird hypothetical one, I'm not sure it's a desirable architecture):

1) If an application can be composed of re-usable building blocks or widgets 2) and those widgets should also be able to contain their own business logic in the form of a redux state machine 3) and that redux state machine should be dynamically integrated into a singleton application state machine

Then I think dynamic action names make a lot of sense. This architecture would allow:

• Multiple instances of reusable "widgets" (think the headerbar or a tool for picking layers on a map) could exist on the same page without separate redux stores
• The singular application redux reducer could actually change as different widgets are added/removed from the page.
• Widgets could be "linked" to other widgets without needing to know before-hand the entire structure of the application.
• Users could "compose" their own complex applications by including and wiring up different "widgets" for their use case (no coding required)

I think this "modular" redux doesn't really exist yet, and it might lead to more complexity than it's worth, but it would be an interesting avenue to explore for a theoretical future "app builder" which would allow non-technical DHIS2 administrators to build tailored "custom" applications for their users using a toolbox of pre-built widgets.

[varl]

I think this "modular" redux doesn't really exist yet, and it might lead to more complexity than it's worth, but it would be an interesting avenue to explore for a theoretical future "app builder" which would allow non-technical DHIS2 administrators to build tailored "custom" applications for their users using a toolbox of pre-built widgets.

Something like this seems similar-but-not-quite: https://github.com/erikras/ducks-modular-redux

[amcgee]

Similar but not quite, yeah. Grouping action/creator/reducer definitions is one thing but dynamically composing reducers (replacing the reducer with a new one whenever the widgets on a page changes) and contextually referencing actions is another entirely.

[varl]

...not to bury application state within components (even if the idea is then to "bubble it up" to application state)

If it is known to belong to the application state beforehand, there is no need to go through the entire workflow to end up with that result. Heuristics defer to expertise.

• Multiple instances of reusable "widgets" (think the headerbar or a tool for picking layers on a map) could exist on the same page without separate redux stores

• The singular application redux reducer could actually change as different widgets are added/removed from the page.

• Widgets could be "linked" to other widgets without needing to know before-hand the entire structure of the application.

I think these three features are something we want in the future, so I do find it interesting. I do not think that we should optimize for this before we have a uniform application structure and established best practices. It will be easier to reason about dynamic behavior and stores at that point.

[Mohammer5]

@amcgee

I think a good test for this is that a redux state snapshot should be something you could use to reproduce a bug.

I couldn't agree more to this. I always used the following rule: If you want to change the view technology (e. g. replacing react with vue), the state should already contain everything you need to build a functional UI (obviously this way the state doesn't include UI state)

@varl

I do not think that we should optimize for this before we have a uniform application structure and established best practices.

100% agree

[amcgee]

think these three features are something we want in the future, so I do find it interesting. I do not think that we should optimize for this before we have a uniform application structure and established best practices. It will be easier to reason about dynamic behavior and stores at that point.

Definitely, wasn't advocating for doing it now just a potential use case for dynamic action naming in one possible glorious future.

👍

[ismay]

The discussion on keeping all the state in redux is something I'm a bit on the fence about though. To me it sounds like we're trying to find an absolute yes or no for something where the answer seems to be "it depends".

The redux docs don't prescribe one or the other: https://redux.js.org/faq/organizing-state#do-i-have-to-put-all-my-state-into-redux-should-i-ever-use-react-s-setstate

One of the react core devs even states that local state makes a lot of sense in certain cases: https://twitter.com/acdlite/status/1045362245507506176?lang=en

So many update performance problems in React go away when you use local state. If you have some state that updates frequently and needs to be fast (ahem, forms), probably shouldn't be putting that in a global store that the whole app subscribes to!

Or am I misunderstanding the discussion?

[amcgee]

I think react state is crucial, we should not keep all state in redux! It's tough to draw the line, but distinguishing between application state (in redux) and interface state (in react) should yield less redux state and therefore hopefully less app complexity.

On Mon, Feb 25, 2019 at 12:33 PM ismay notifications@github.com wrote:

The discussion on keeping all the state in redux is something I'm a bit on the fence about though. To me it sounds like we're trying to find an absolute yes or no for something where the answer seems to be "it depends".

The redux docs don't prescribe one or the other: https://redux.js.org/faq/organizing-state#do-i-have-to-put-all-my-state-into-redux-should-i-ever-use-react-s-setstate

One of the react core devs even states that local state makes a lot of sense in certain cases: https://twitter.com/acdlite/status/1045362245507506176?lang=en

So many update performance problems in React go away when you use local state. If you have some state that updates frequently and needs to be fast (ahem, forms), probably shouldn't be putting that in a global store that the whole app subscribes to!

Or am I misunderstanding the discussion?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dhis2/notes/issues/18#issuecomment-466994629, or mute the thread https://github.com/notifications/unsubscribe-auth/AA52sCVl06Gaa44ASaCAdriDht9TwDWPks5vQ9f_gaJpZM4bJepJ .

[ismay]

I think react state is crucial, we should not keep all state in redux! It's tough to draw the line, but distinguishing between application state (in redux) and interface state (in react) should yield less redux state and therefore hopefully less app complexity.

Allright, yeah that's basically how I perceive it as well 👍