Add flow typed Redux actions & refactor actions

[AWolf81]

As mentioned in #28 it would be great to have typed Redux actions and I think src/actions/index.js could be refactored as well. We'd like to do this in a separate PR to change everything at once.

I'd separate each related action into it's own file and export just the type Action from src/actions/index.js.

Example code

src/actions/project.actions.js

export const SAVE_PROJECT_SETTINGS_START = 'SAVE_PROJECT_SETTINGS_START';
export const SAVE_PROJECT_SETTINGS_ERROR = 'SAVE_PROJECT_SETTINGS_ERROR';
export const SAVE_PROJECT_SETTINGS_FINISH = 'SAVE_PROJECT_SETTINGS_FINISH';

export type saveProjectSettingsAction = {
    type: SAVE_PROJECT_SETTINGS_START,
    project: Project,
};

export type doneProjectSettingsAction = {
    type:  SAVE_PROJECT_SETTINGS_FINISH,
    project: Project,
};

export type ProjectAction =
    | saveProjectSettingsAction
    | doneProjectSettingsAction;

export const doneProjectSettings: doneProjectSettingsAction  = (project: Project) => ({
    type: SAVE_PROJECT_SETTINGS_FINISH ,
    project
 });

src/actions/index.js

export type { ProjectAction } from './project.actions.js';
export type { DependencyAction } from './dependency.actions.js';

Usage in a middleware project.middleware.js:

import type { ProjectAction } from '../actions';
import { SAVE_PROJECT_SETTINGS_START, doneProjectSettings } from '../actions/project.actions'
export default (store: any) => (next: any) => (action: ProjectAction) => {
   switch(action.type) {
     case SAVE_PROJECT_SETTINGS_START:
         // rename folder
         // change project name & icon in package.json
         // once resolved...
         store.dispatch(doneProjectSettings(action.project));
         break;
   }
  return next(action);
}

Topics to discuss

• What needs to be exported from src/actions/index.js? Just the types or anything else?
• Create a list of related actions? I'd like to separate actions to individual files because everything inside of src/actions/index.js is hard to maintain and difficult to add new actions. I think actions could be grouped like this (list is maybe not complete):
  • dependency.actions.js
  • projects.actions.js (addProject, refreshProjects, selectProject, createNewProject, ... )
  • task.actions.js
  • modal.actions.js

Additional context

Flow redux actions docs

[joshwcomeau]

This seems like a good approach to me!

What needs to be exported from src/actions/index.js? Just the types or anything else?

I think we still need to export the constants, so that we can avoid a typo in a reducer that means the reducer isn't behaving as we expect... unless there's some way for Flow to catch that? I haven't seen it though.

Create a list of related actions? I'd like to separate actions to individual files because everything inside of src/actions/index.js is hard to maintain and difficult to add new actions.

Hm, I'm curious why you think it's hard to maintain? Just because of the general difficulty in navigating large files?

The thing is that actions shouldn't correspond neatly to reducers. For example, creating a new project affects the projects.reducer.js, but also dependencies.reducer.js and tasks.reducer.js.

A common mistake (at least, it's a mistake IMO) that I see is folks assuming that every action can only be tackled by 1 reducer. Redux feels much nicer to me when actions are based on what the user is doing, and aren't specifically tied to a reducer. Dan Abramov has said that this is exactly why actions exist, and we don't simply have methods that directly update the reducer; it's a helpful abstraction.

I also like that there's 1 place I can check to scan and see all the actions that the app supports. It's a great way to get up to speed quickly on what actions a user can take in a project.

I'm not so concerned about long files... IMO long files are often bad because length and complexity are often correlated. A 1000-line React component is almost certainly a bad thing, because it means that far too many things are happening inside a single component. But, actions are super simple (and should become even simpler if we can get rid of the thunks). So I don't think 1000 lines of actions is a code smell, since it's just a big collection of simple things.

There is the fact that long files are harder to navigate. At least right now, the file doesn't seem that long, but if the Flow refactor makes it much longer, we could split up the types (so an actions.js, and an actions.types.js)? I'm not sure I love that idea, though: it means you always have to change 2 files when updating an action.

If we can find a way to split it up into multiple fines that doesn't lead us down the road of coupling actions to reducers, I'd be interested! But in the meantime I think one big file is the best way =)

[AWolf81]

Hard to maintain is probably not the right word. I think it's easier to find where to add new action - so it's more a style that helps to add new things to the right place/line.

Sure you can add it into one file and group related actions by comments, so you can add new related actions to the right place.

I don't think that grouping actions into different files will affect how they're used - you just need to split the imports to different files. But you're probably right that it looks like the actions could be only used in the reducer with a similar name.

So I'm also OK with just converting the src\actions\index.js to Redux with Flow. I just thought that it would help and is easy to do as every action needs to be touched & checked.

[joshwcomeau]

Hard to maintain is probably not the right word. I think it's easier to find where to add new action - so it's more a style that helps to add new things to the right place/line.

Sure you can add it into one file and group related actions by comments, so you can add new related actions to the right place.

Ahh, gotcha. Yeah, it can be hard to figure out where to add new actions. I'm not too worried about this; in the worst case, someone might accidentally duplicate an action, but we'd probably catch that in code review, and it'd be easy to fix. For now I think comments and rough grouping should be OK - if we see that it happens a lot, that there's a good amount of confusion with actions, I'm 100% open to revisiting the idea :)

I don't think that grouping actions into different files will affect how they're used - you just need to split the imports to different files. But you're probably right that it looks like the actions could be only used in the reducer with a similar name.

Yeah, so you're right, it isn't a hard limit... but I think people naturally have a tendency to treat actions as "setters", especially when coming from an MVC framework. So I think the patterns we adopt should help guide folks to the patterns we want, and I feel like splitting things up by "type" gives the wrong impression... plus sometimes it might actually be hard to figure out which file something goes in (although admittedly I can't think of any examples).

Incidentally, if you (or anyone else reading this) wants me to elaborate more on my redux philosophy and why I think it's better, let me know. I've wanted to write a blog post about it, and if such a post could be helpful for Guppy maintainers, it'd push it up on my priority list.

[AWolf81]

👍 A blog post about your Redux philosophy sounds great and is a good way to learn more about Redux.

Will you also cover thing like - should everything be inside Redux state or is a mix between local states and Redux the way to go? I'm prefering to keep thing local and use Redux to share state if multiple components needs access to the state. It would be also interesting how and when the context api should be used. I played a bit with the context api but I need to learn/read more about it.

[joshwcomeau]

Will you also cover thing like - should everything be inside Redux state or is a mix between local states and Redux the way to go? I'm prefering to keep thing local and use Redux to share state if multiple components needs access to the state. It would be also interesting how and when the context api should be used. I played a bit with the context api but I need to learn/read more about it.

Yeah, so my blog post would cover all of this. To give you some quick answers here, though:

• Definitely use React local state when it makes sense! The reasons to use Redux are if the state will be needed widely across the app, or the state transformations are complex and we want the introspection/time-travel benefits of Redux. Or if the state changes involve a lot of side-effects and we want the benefit of Redux middleware.

• React Context is great at solving that first reason, sharing simple state across the app, but doesn't help with the second concern, managing complex state transitions or side-effects. Modal state would be great for React Context, since it can sometimes be widely used (if a component way down the tree needs to trigger opening a modal), but the state transformations are super simple (just change a string that represents which modal is open).