Closed timdorr closed 5 years ago
YES YES YES! I HAVE MANY OPINIONS AND WISHES FOR THIS! But I'll restrain myself and let the discussion get going some first.
One wishlist item I'd have that I'm not sure is feasible is built-in HMR for reducers automatically added in a createReduxStore()
function. Unfortunately, my knowledge of Webpack and some experimentation says that's not very feasible, because you have to have hardcoded paths to reducer files in the Webpack module.hot.accept()
callbacks. Not sure how Parcel or other bundlers handle that.
So, initial request for feedback from everyone:
Lets name 3 key points, where possible variants will be isolated from each other.
First point. Reducer managers 1.0 Vanilla (it IS the case here?) 1.1 Immer.js 1.2 Coredux 1.3 100500 more packages
Second point. Middlewares 2.0 Vanilla (is it the case?) 2.1 Sagas 2.2 Epics 2.3 100500 more packages.
Third point. React integration 3.0. Vanilla (it IS the case here?) 3.1. Subspace 3.2 Restate 3.3 100500 more packages.
4th, bonus, point 4.0 Testing all this stuff in a boilerplate.
It is impossible to pick "the chosen ones", for example I love and use both Sagas and Epics, but possible to provide some "cooking receipts" and recommendations for combinations.
For example most of redux users aware only about middlewares. A small part also doing something with reducers(ok lots heard about immutable.js), just a few ones enhance React integration.
Starter Kit could just help build for full and rich approach from the beginning. Or one need more that one Starter Kit.
Asked for feedback on setup/boilerplate pain points on Twitter: https://twitter.com/acemarke/status/969040835508604929
Copying my response on twitter:
I’ve explicitly stayed away from adding redux boilerplate as IMO it adds complexity when introducing ‘your way’ of doing redux to another person. I would love more prescriptive conventions enforced via code to make redux a more portable skill.
To that end, having a create-redux-app
style repo - however designed more as a library than a framework - especially one explicitly endorsed by Redux maintainers of best and recommended practices, I think would go a long way to solidifying and adding clarity on proper Redux usage.
We've been talking a bit about this on Reactiflux, wanted to copy over a consolidation of desires for inclusion in this thread, some of this overlaps with the points you already listed @timdorr (so it's an enthusiastic confirmation).
Why not officially backing something that already exists, like Henrik Joreteg's redux-bundler?
https://github.com/HenrikJoreteg/redux-bundler
It's concise, opinionated (in a good way), allows for reuse of redux-related functionality across applications and still gives a nice solution for async handling.
It's a fascinating-looking project, and one that I really want to investigate further, but it's also out of scope for what I want to do with this task.
What I really want to accomplish here is:
I want to keep this effort fairly tightly scoped. I've been chatting with @hswolff and @nickmccurdy over in Reactiflux the last couple hours, and while the discussion has been great, we've already run through a dozen overlapping use cases and ideas that could really over-complicate things.
I want to get something useful, valuable, and viable put together without too much bikeshedding, and then have other discussions off to the side (like an API for making Redux "bundles" or "modules", etc).
To kick off some initial boilerplating of APIs here's a basic idea for 1-3 of my list I posted above:
// #1
export function createStore({
// If an object is given it's passed to combineReducers
// otherwise it's just used directly.
reducer: Object<String, Function> | Function,
// Middleware is built-in and accepts an array of middleware functions.
middleware: Array<MiddlewareFunction>,
// Built-in support for devtools.
// Defaults to NODE_ENV !== production
devTools: boolean,
// Same as current createStore.
preloadedState,
// Same as current createStore.
enhancer,
}) {};
// Re-exported for easy usage.
export function combineReducers() {}
// #2
// Re-export immer as the built-in immutability helper.
import immer from 'immer';
export const createNextState = immer;
// #3
// Export an already written version of createReducer .
// Same thing as https://redux.js.org/recipes/structuring-reducers/refactoring-reducers-example#reducing-boilerplate
export function createReducer() {}
Yep, I dig that. I'd then expect that it would either always add thunk()
at the front of the middleware list, or possibly only if you didn't supply any middlewares yourself. We might not even want to allow the user to specify enhancers here. We'd add applyMiddleware()
automatically if there's any middleware, and use the composeWithDevtools
function from redux-devtools-extension
if devTools : true
. For purposes of this "easy setup" thing, I think we may want to eliminate "enhancers" from the things the user can configure.
+1 on eliminating enhancers.
On your point of thunk being automatically added, we should be able to have our cake and eat it too via exporting the function we'd use to add default middleware:
export function createDefaultMiddleware(...additional) {
return [thunk(), ...additional],
}
// Within a user's application
createStore({
// By default it's just createDefaultMiddleware()
// However if you want to add any other middleware you can.
middleware: createDefaultMiddleware(any, other, middleware)
})
Yeah, that's roughly what I was thinking. My main concern is figuring out if the user accidentally tries to supply their own instance of thunk
.
I decided to play around with things a bit this evening. Here's a first cut. Thoughts?
// configureStore.js
import {createStore, compose, applyMiddleware, combineReducers} from "redux";
import { composeWithDevTools } from 'redux-devtools-extension';
import createNextState from "immer";
import isPlainObject from "./isPlainObject";
import thunk from "redux-thunk";
const IS_DEVELOPMENT = process.env.NODE_ENV !== "production";
export function createDefaultMiddleware(...additional) {
return [thunk, ...additional];
}
export function configureStore(options = {}) {
const {
reducer,
middleware = createDefaultMiddleware(),
devTools = IS_DEVELOPMENT,
preloadedState,
} = options;
let rootReducer;
if(typeof reducer === "function") {
rootReducer = reducer;
}
else if(isPlainObject(reducer)) {
rootReducer = combineReducers(reducer);
}
else {
throw new Error("Reducer argument must be a function or an object of functions that can be passed to combineReducers");
}
const middlewareEnhancer = applyMiddleware(...middleware);
const storeEnhancers = [middlewareEnhancer];
let finalCompose = devTools ? composeWithDevTools : compose;
const composedEnhancer = finalCompose(...storeEnhancers);
const store = createStore(
rootReducer,
preloadedState,
composedEnhancer
);
return store;
}
export function createReducer(initialState, actionsMap) {
return function(state = initialState, action) {
const {type, payload} = action;
return createNextState(state, draft => {
const caseReducer = actionsMap[type];
if(caseReducer) {
return caseReducer(draft, payload);
}
return draft;
});
}
}
export {createNextState, combineReducers};
And the obligatory example todo app:
import {configureStore, createReducer} from "./configureStore";
const ADD_TODO = "ADD_TODO";
const TOGGLE_TODO = "TOGGLE_TODO";
const SET_VISIBILITY_FILTER = "SET_VISIBILITY_FILTER";
function setVisibility(state, newFilterType) {
return newFilterType;
}
const visibilityReducer = createReducer("SHOW_ALL", {
[SET_VISIBILITY_FILTER] : setVisibility
});
function addTodo(state, newTodo) {
state.push({...newTodo, completed : false});
}
function toggleTodo(state, payload) {
const {index} = payload;
const todo = state[index];
todo.completed = !todo.completed;
}
const todosReducer = createReducer([], {
[ADD_TODO] : addTodo,
[TOGGLE_TODO] : toggleTodo
});
const preloadedState = {
todos: [{
text: 'Eat food',
completed: true
}, {
text: 'Exercise',
completed: false
}],
visibilityFilter : 'SHOW_COMPLETED'
};
const store = configureStore({
reducer : {
todos : todosReducer,
visibilityFilter : visibilityReducer,
},
preloadedState,
});
const exercise1 = store.getState().todos[1];
store.dispatch({type : "TOGGLE_TODO", payload : {index : 1}});
const exercise2 = store.getState().todos[1];
console.log("Same object: ", exercise1 === exercise2);
store.dispatch({type : "SET_VISIBILITY_FILTER", payload : "SHOW_COMPLETED"});
console.log(store.getState());
I think this is a solid start and a great beginning to the lib being discussed in this thread. It covers points 1,2,3, and 6 from my list of desired feature above.
Iterating on top of this would be great, but this is a solid foundation.
Rather than focusing on the implementation, I'd like us to focus on the DX. What are the exports/APIs and how are they used? I think it's a great chance to get past just the configuration and also look at how folks use the store. Some random ideas:
Make it super-easy to pair up your reducers and actions along with their state output. This is rough, but here's the basic idea:
import { createStore, createActionPack, combineActionPacks } from 'redux/starter'
const counter = createActionPack({
increment: counter => counter + 1,
decrement: counter => counter - 1,
set: (_, value) => value
}, 0)
const posts = createActionPack({
load: async () => await fetch('/posts'),
create: async (state, text) => state.push(await fetch('/posts/new', { body: { text } }))
}, [])
const store = createStore(
combineActionPacks(
counter,
posts
)
)
store.dispatch(counter.increment())
store.dispatch(counter.set(42))
await store.dispatch(posts.load())
await store.dispatch(posts.create('First!'))
This one I'm not sold on by any means, but I was thinking back to the days when we had history singletons in React Router. It wasn't perfect, but it provided some value.
import { store, configureStore } from 'redux/starter'
configureStore(reducer, middleware, initialState)
store.dispatch(someAction())
store.getState()
import
I'll save my wackiest for last. But what if we allowed configuration by importing files. We'd need some behind the scenes global passing, which gets messy/troublesome. But again, the focus is on the user, not our side.
import { createStore } from 'redux/starter'
import 'redux/starter/devtools'
import 'redux/starter/logger'
import 'redux/starter/immutable'
// OR!
import { createStore } from 'redux/starter/developer'
import { createStore } from 'redux/starter/production'
For that second example, I can see users either aliasing to that package in their build tooling or doing something like what React does.
OK, that's it from me for now. Again, I want to be thinking user-first. Whatever dirty, ugly hacks we have to make to implement this is irrelevant. The whole point is a great DX with less code written and less configuration needed.
Love these ideas thus far. My only wish for whatever this turns out being would be graceful access to the lower level, standard Redux api. There needs to be a natural path for people to customize or eject. Whether that is in a few days, or a year or two, it should be simple. Lock-in is one of the problems I see (from my own attempt at this with Jumpstate and from more recent libraries like Rematch). The more that we can avoid that, the better taste people will have in their mouth :)
Yeah, I've seen quite a few util libs that do stuff like that "action pack" (at least as far as the "write some reducers, turn the keys into action types and action creators" aspect), and I was considering throwing that in as an additional item - just hadn't gotten there yet. Not sure about the "combineActionPack" aspect.
Had some additional discussion with @hswolff last night. I'd like to consider adding/re-exporting the selectorator library, which appears to be a superset of the Reselect API with some additional useful benefits:
import createSelector from 'selectorator';
// selector created with single method call
const getBarBaz = createSelector(['foo.bar', 'baz'], (bar, baz) => {
return `${bar} ${baz}`;
});
That would help cut down on a lot of the hand-written "simple input" selectors, like const selectTodos = state => state.todos
.
I'm not sure I see the benefit of adding a selector library. We would have to also be providing some opinion about state shape that is enabled by it.
It's at least partly just so there's one less item the user has to install into package.json
.
I think a selector library would be awesome.
Also relevant: the recent article on the apollo-boost
starter kit
If we can extend react-redux
's connect
's API to use reselect (or a similar selector library) when the mapStateToProps
is an Array
, we could encourage the use of selectors and make it very convenient without breaking back compatibility. I don't think this library should be coupled with React or react-redux
necessarily, but maybe we could handle that extensibility through an option for react-redux
or provide another package wrapping the starter kit with react-redux
support.
Yeah, I'm not looking to do anything with React-Redux for this initial MVP.
In that case is there a good way to add an abstraction for selectors at the Redux level or is this something we could only do effectively on top of react-redux? The problem is we wouldn't want this to be incompatible with react-redux, so I guess if that's not possible we should ditch selectors for now.
As long as it implements the store API, then it's compatible with react-redux. We're not looking to break that API, just improve DX.
That makes sense. I haven't used selectors outside of react-redux so I was wondering if they would be applicable to configuring Redux itself here.
@timdorr +1 on the desire expressed with easier reducer/action pairs. I would also love to introduce some way to make it easier to create a redux module (aka the ducks pattern) such that creating a reducer, action creators, and selectors is a much more pleasant and easier to use experience.
I do think finding a solution to that problem that makes the majority of people happy will require a little bit more bike shedding and I'm wondering if it's something we can add in a follow-up version so that it doesn't block an initial release.
I think what @markerikson created in his comment already reduces a good deal of boilerplate and would make for a great initial pre-release.
After that, we can tackle the other aspects of Redux that are cumbersome (redux/action/selector creation).
I think trying to solve all problems up front is going to get in the way of making any form of tangible progress.
If you think it's ready enough, I think it would be a good idea to start a repository so we can test out this setup with example code and dogfood it a bit (or maybe a package in a feature branch of the monorepo, and we could collaborate through PRs).
Sure. I'll see if I can get something set up a bit later today.
We don't need another repository. This will exist within the current one. Please make anything a PR first.
Should this be a part of the main redux
package as a separate import (so this is completely back compatible) or would you prefer a monorepo with multiple packages? I think the former would be enough, we could always use another file export without creating a new package if it gets too big.
I'd kinda like to play around with this as a separate repo/package to start with, really.
It may be easier to use the existing tooling setup in this repo since merging it back in the future could be error prone. We also probably don't need different syntax support because we're not making this React specific.
If it's going to be one single file with some pre-configuration, then keeping it here would be easiest. Because of the dependencies, it should be a separate package. We don't want to saddle existing apps with those.
Again, I don't think we need to go monorepo, but we can have a folder for specific packaged builds if we start building more than one. Just a starter
folder for now with its own package.json should do the trick.
I like the idea of keeping this local to the main redux package if it's only using code local to redux itself. I get more hesitant if we start to pull in external dependencies to support the ideas expressed here.
I think some external dependencies like immer, thunk/promise/saga, and reselect would be invaluable to our goals here and worth the extra dependencies for the starter, though we may not want those dependencies on the main redux
package itself.
So, uh... I went and did a thing:
Things that are different from the snippet earlier:
enhancers
config option, just becauseselectorator
dependency and exported createSelector
Thoughts?
As someone that just taught myself Redux (and React) in the last couple weeks, I can't wait to see the final product. I'm sure I'm going to have a ton of stuff to clean up in my code.
😏 👍
So, as long idea is to simplify redux usage, I could propose 2 moments:
Add build-in function to literally expect(mapStateToProps(state)).toEqual(mapStateToProps(state))
. Just to encourage people to create a better code. I never saw tests like this, but not-very-pure mapStateToProps is a one of redux problems.
Consider adoption something like memoize-state. It is like Immer.js for selectors. The key point - the customer could just write mapStateToProps or selector in the form he prefers, and get it memoized. It even could solve this reselect issue with "Sharing Selectors with Props Across Multiple Component Instances" without loosing "common" memoization between instances. Just double memoize the stuff
Still, as we discussed above, this is more of a react-redux thing than something that would be in the starter package which is agnostic to React.
This is really cool!
I have one request / bit of feedback, though; I'd rather if Immer was not used implicitly.
Immer is great. It's become one of my favorite utilities. But even when using it explicitly (produce(baseState, draftState => ...
), I worry about when other people who aren't familiar with it view my code, and don't realize that I can only do this mutative stuff because of Immer superpowers.
The way the API looks now, I can imagine newcomers not realizing that redux state should be immutable. I think that misunderstanding would bite them hard when they try and use Redux without this starter kit.
I see that createNextState
is already an export, so why not encourage users to use it explicitly:
import {createReducer, createNextState} from "@acemarke/redux-starter-kit";
function addTodo(state, newTodo) {
return createNextState(state, draftState => {
draftState.push({...newTodo, completed : false});
});
}
(apologies if I've misunderstood how createNextState
is intended to be used! Haven't dug into it at all)
See https://github.com/markerikson/redux-starter-kit/issues/5.
The way the API looks now, I can imagine newcomers not realizing that redux state should be immutable. I think that misunderstanding would bite them hard when they try and use Redux without this starter kit.
In my opinion, we should recommend against using Redux directly in favor of this package when it is complete because of https://github.com/reactjs/redux/issues/2858. Basically we want to either warn or safeguard users from all mutations by default, but we can't easily do this in core without breaking things, so we should recommend against using core except in edge cases where the starter kit is not sufficient. This is similar to how React recommends CRA and Redux recommends react-redux when using React.
I see that createNextState is already an export, so why not encourage users to use it explicitly:
Not a bad idea, but I think we're more in favor of being implicit here especially because we're not generating source code and the addition of immer is back compatible.
I see a lot of people recommending to use libs
like immer.js
, which from my humble opinion consists in unwittingly deviate the motivation of learning and respecting the store
requirements.
We all know that the store can't be mutated, thus, I'd much rather have a validating mechanism where it checks the newState
instance against ( ===
) the previousState
and if it's the same instance throw an error.
This way everybody would be enforced to learn the store
's rules and requirements.
PS: I truly understand the benefit of immer
and I have absolutely nothing against it, but we are reaching a time where people are finally understanding the concept, it'd generate a huge source of confusion, and technically saying, if we benchmark it, we'd be doing nothing but adding an extra layer of complexity just to loosen up the library.
Personal Suggestions:
The logger
, devTools
and Thunk
should be added as default of the Starter
, the rest I believe is more of a personal choice.
We all know that the store can't be mutated, thus, I'd much rather have a validating mechanism where it checks the newState instance against ( === ) the previousState and if it's the same instance throw an error.
There are cases where depending on the action you wouldn't want the state to change, so you'd return the same store instance to save memory and prevent any false positive diffs. Even if we got around this issue it seems like Redux doesn't want a validator in core and I think Immer is an okay safeguard for this personally. I do agree that it's kind of defeating the purpose of learning Redux to an extent, but I see so many developers learning Redux that don't want to learn pure FP or aren't aware of it.
if we benchmark it, we'd be doing nothing but adding an extra layer of complexity just to loosen up the library.
As far as I'm aware, the way that Immer is implemented the Proxy only has performance overhead if it's actually mutated, so typical immutable update reducers shouldn't have a performance overhead. I think this is an okay balance.
The logger, devTools and Thunk should be added as default of the Starter, the rest I believe is more of a personal choice.
There are some other things I'd like to consider removing (though I personally don't see a huge advantage to the logger over the developer tools that makes up for its performance overhead), see https://github.com/markerikson/redux-starter-kit/issues/19. However I'm not against adding more things if they're back compatible like our current usage of Immer and Thunk.
There are cases where depending on the action you wouldn't want the state to change, so you'd return the same store instance to save memory and prevent any false positive diffs. Even if we got around this issue it seems like Redux doesn't want a validator in core and I think Immer is an okay safeguard for this personally. I do agree that it's kind of defeating the purpose of learning Redux to an extent, but I see so many developers learning Redux that don't want to learn pure FP or aren't aware of it.
@nickmccurdy I totally understand your point of view, and I do agree that we shouldn't have a learning obstruction due to some concepts of FP, Mutation and etc... But as an educator I strongly believe in not sugar coating the apprentice by hiding the complexity from them. Anyway, it's just a matter of taste, I am a pro learning and forwardness, and looking at this proposal, my instincts somehow tells me that we'd be moving forward with acceptance, but regressing with standards. Just in note, think of all content in the web right now teaching Redux Standards and how it's finally picking up.
As far as I'm aware, the way that Immer is implemented the Proxy only has performance overhead if it's actually mutated, so typical immutable update reducers shouldn't have a performance overhead. I think this is an okay balance.
When I mean adding a layer of complexity, I mean Immer
has never been there and now it could possibly be, we'd have to support it in the library, test against, evaluate perfomance issues and last but not least this section here, makes me apprehensive:
Immer.js - Auto freezing Immer automatically freezes any state trees that are modified using produce. This protects against accidental modifications of the state tree outside of a producer. This comes with a performance impact, so it is recommended to disable this option in production. It is by default enabled. By default it is turned on during local development, and turned off in production. Use setAutoFreeze(true / false) to explicitly turn this feature on or off.
And all of a sudden now we also need to support features added in Development / Prod
. It's just a good amount of work and worrisome for not much to trade off, but again, this is just my personal opinion.
@codedavinci : the point of this redux-starter-kit
library is to add useful tools and abstraction on top of the "basic" Redux store setup process. Also, note that this is not going into the Redux core itself, but rather into a separate library that we would like to make an "official" part of the Redux org eventually.
All your existing Redux code will work. All existing tutorials will still be fine. We're just trying to add an officially supported simpler approach to setting up and using Redux.
@markerikson I understand the goal of having the starter kit and I am fully down for everything mentioned above, just a bit resistant of having a lib like Immer.js
that changes the concept, of the library, people will eventually learn the starter-kit
patterns and fully ignore the core concepts. No store mutation is the motto, Redux's been always selling it, it's part of the brand.
PS: I fully understand that Immer.js
doesn't mutate anything, but rather helps you not to, I am only and just talking about syntactically standards allowed such as push
, splice
...
Maybe, It's just myself being resistant against changes :)
@codedavinci : yeah, I understand the point of the concerns. Still, one of the common complaints about Redux is the pain of writing proper immutable update code, and Immer is simply the best library I've seen for simplifying those updates.
@markerikson, I actually did some digging into the immer
and I noticed it's very light, simple and efficient. I am actually sold :) .
Which repo are you drafting things up ? those:
https://github.com/markerikson/redux-starter-kit https://www.npmjs.com/package/@acemarke/redux-starter-kit https://unpkg.com/@acemarke/redux-starter-kit@0.0.1/
Based on comments over in #2858 and earlier in #2295, it sounds like a preconfigured starter kit of Redux core + some common middleware + store enhancers + other tooling might be helpful to get started with the library.
I've started by reserving a package name (I'm not dead set on it, though) and we can start to fill in the blanks there. I would like to maintain the contents of that package here, so we don't have to set up another repo or anything. We may want to investigate a monorepo, but I don't think there's a pressing need currently.
One thing I'd like to investigate at the same time: splitting
combineReducers
to its own package. It's the one thing that gets prescriptive about how to structure and operate a store. It's obviously not the only way, but I worry many folks don't see past it.As for what goes in a starter package, the short list in my mind includes one of the boilerplate reduction libraries (or something we build), popular middleware like thunks and sagas, and helpful dev tooling. We could have a subset package for React-specific users. In fact, I started a Twitter poll to find out if that's even necessary (please RT!).
I don't think we need to build a Create React App-like experience with a CLI tool and all that jazz. But something out of the box that gives a great dev experience with better debugging and less code.