tj / frontend-boilerplate

webpack-react-redux-babel-autoprefixer-hmr-postcss-css-modules-rucksack-boilerplate (unmaintained, I don't use it anymore)
2.93k stars 268 forks source link

Functions as constants #6

Closed ashaffer closed 8 years ago

ashaffer commented 8 years ago

I'd be curious what you think about this. I've been overriding Function.prototype.toString so that I don't need to create a separate set of constants, which somewhat significantly reduces the headache of creating new actions. I have made a small utility for doing it:

create-action

tj commented 8 years ago

Sounds decent to me! If you have any other suggestions let me know, still new to the react/redux world of things, I definitely prefer fewer abstractions since there's already so many to deal with but less redundancy there is nice

tj commented 8 years ago

How is this different than https://github.com/acdlite/redux-actions? kinda might as well just use strings since js is so flaky type-wise anyway. I could see it for IDE support, not sure if there's something decent for sublime

ashaffer commented 8 years ago

They are identical, with the exception that mine adds a .toString() on the action creator, so that you can also use it as the constant.

E.g.

const addTodo = createAction('ADD_TODO')

const reducer = handleActions({
  [addTodo]: handleAddTodo
})

This saves you the headache of having to update a constants file and an action file every time you want to add a new action.

tj commented 8 years ago

cool might use that if I can get babel to enforce constants, pretty much just as effective as strings otherwise

ashaffer commented 8 years ago

I should also add, I created variants of the other reducer helpers, and some new things, as well:

compose-reducers - Run two reducers at the same level. i.e. reducerA(reducerB(state, action), action)

combine-reducers - Identical to the one exported by redux, but doesn't do redux's sanity checks. It's sanity checks prevent you from composing two reducers that access the same fields. e.g. composeReducers(reducerA, combineReducers({todos: todoReducer})). This is sometimes desirable if you have certain reducers that transform the entire state atom (e.g. rehydrating from the server / localstorage)

handle-actions - Identical to the one in redux-actions, except it passes your handler payload rather than the entire action. This makes everything much nicer, because you can use simple primitive functions for almost everything (reduce-replace is another micro library that makes this even easier).

reduce-key - Lets you reduce an item in a keyed collection (e.g. a particular todo in the list).

You can see an example of all of these things working together in vdux's todo example as well if you'd like. Apologies for dumping so many things on you all at once here.

aweary commented 8 years ago

redux-actions is more feature rich. I'd recommend using it over something simpler like create-action, as it provides things like FSA compliant error handling and other helper functions like handleAction for reducers.

ashaffer commented 8 years ago

@Aweary I have created variants of the other features as well, though less the validation and error handling, if you look at my most recent comment above yours.

aweary commented 8 years ago

Sorry, missed that. I still think redux-actions has the advantage here, specifically in that it provides essentially the same functionality (the differences coming down to what is essentially personal preference) within a single dependency, and also has the benefit of being pretty widely used in the community (I think thats important for a boilerplate repo IMO, especially since there's not a really good reason not to use it).

Like you said, it also already has the validation and error handling as well.

tj commented 8 years ago

switched to handle-actions, fits the average use-case of just wanting at the payload I think. Part of me just prefers a simple switch TBH haha but meh, works for now

aweary commented 8 years ago

handle-actions is explicitly non-compliant with handling FSA-compliant actions

This means your reducers will be unable to examine any other fields (e.g. meta) - if you want to use other fields, don't use this module.

I don't think it's a good idea to strip the options of accessing the error or meta fields by default.

tj commented 8 years ago

When would you use them in reducers? Not saying you're wrong, just curious. From my limited use so far error/meta has only really been useful in middleware.

ashaffer commented 8 years ago

@Aweary Ya, there is a good argument to be made for it on the basis of its commonness. I suppose my proposition is that the way mine work are better (generate less boilerplate, more accurately model the problem, etc) and should become the more common implementations :). Certainly reasonable people can disagree here though.

As to the bundling issue - I have found that annoying myself in using my own functions and I may create a bundle soon (or anyone else is free to create their own of course). I think that it makes sense to implement the underlying things as independent modules and then create separate bundles if people want - that way people can always mix and match and obtain exactly the functionality they want.

In regards to stripping of the other fields: To me, meta is something that should only be handled by middleware, and error-related things should just be their own actions that indicate them as such, not given some special status by the FSA implementation. That's why I didn't implement that stuff or the validation.

@tj Ya, a simple switch is pretty appealing to me too. For something todomvc sized it just barely still makes sense, I think. I tried to use them for a time, but things get messy pretty quickly unfortunately and you end up having to rewrite even more boilerplate every time you want to add something.

tj commented 8 years ago

I'll revert for now. It would be awkward to have a mix of the two in a larger app so maybe it's not the best default.

aweary commented 8 years ago

Error handling might occur in the reducer, depending on the implementation. Semantically speaking, meta can really be any descriptor that isn't considered a payload for the action, so it could also be important to state composition.

In regards to stripping of the other fields: To me, meta is something that should only be handled by middleware, and error-related things should just be their own actions that indicate them as such, not given some special status by the FSA implementation.

While I agree with you on a personal level, that's just a matter of preference really. It's not incorrect either way, but I would say that not stripping the fields leaves it open for people to use either way.

tj commented 8 years ago

I guess reverting optimistic state changes would be a good use-case, I've just used middleware so far

ashaffer commented 8 years ago

@Aweary Ya, I think it's a matter of convenience and taste. Stripping the fields for handle-actions certainly doesn't prevent you from using plain functions with switches in addition, so i'm not sure it matters a whole lot that it prevents you from accessing those things - you can always use a regular function in addition.

While I agree with you on a personal level, that's just a matter of preference really. It's not incorrect either way, but I would say that not stripping the fields leaves it open for people to use either way.

I actually kind of disagree with you here. I think it overloads the representation of the action itself, and makes it ambiguous what actually happened. E.g. when is an error considered part of the action? Did the action also complete with some partial payload? It smells wrong to me in the same way as in-band signalling - it forcefully mixes concerns in a way that doesn't necessarily match the problem domain.

@tj What do you mean by using it for revering optimistic state updates? How would that work?

tj commented 8 years ago

@ashaffer you could update the UI, fire off a request, and revert on failure etc. Other than cases like that I don't see when you'd use it in a reducer, maybe I'm missing something

ashaffer commented 8 years ago

@tj Ah, ya I don't think there is anything wrong with handling errors in reducers for something like that. I just feel like they ought to be separate actions, with their own payloads (e.g. REQUEST_FAILED) rather than mixed into the main one. And if you do that, you don't need the error property.

tj commented 8 years ago

Yea same, multiple shapes for a "single" action feels a little weird but whatever!