-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discussion: Roadmap to 1.0 #82
Comments
Dropping
I would consider dropping
Good idea! It also improves support for non-string action types (although these are currently not permitted by the TypeScript typings).
I would generalise "API function" to just "function that returns a promise", but I guess that is what you meant? Like this: const doAsyncStuff = createAsyncAction('doAsyncStuff', () => API.doAsyncStuff())
doAsyncStuff() // returns thunk
doAsyncStuff.started
doAsyncStuff.succeeded
doAsyncStuff.failed I like the idea, and think this pattern is sufficiently widespread to be codified.
That would require some elaborate monkey-patching, right? I wonder if this is worth the strange unforeseen problems it might cause.
Perhaps we should re-export |
@denisw: Thanks for taking the time to add your thoughts here and in #83, as well as the recent TS conversion. I really appreciate it. Given that this is the "Roadmap" thread, I'm going to take this chance to write up my vision for RSK, and how that affects the included functionality. I'll add some specific responses to your comments after that. |
My Vision for Redux Starter KitTL;DR:
BackgroundThere are several factors that combine to make it relatively hard to use Redux in comparison to other options:
In addition, a lot of people really hate doing any more physical typing of characters than the absolute minimum they can get away with. A couple years ago, I filed Redux issue #2295: Request for Discussion: Redux "boilerplate", learning curve, abstraction, and opinionatedness to discuss ways we could improve things. The thread wound up doing a lot of bikeshedding (naturally) and eventually devolved into people linking their own Redux abstraction libraries they'd created. However, the very first comment from @modernserf was pretty much on-point:
That was the initial genesis for Redux Starter Kit. ObjectivesPer the README, other specific inspirations are Create-React-App and Apollo-Boost. Both are packages that offer opinionated defaults and abstractions over a more complex and configurable set of tools, with the goal of providing an easy way to get started and be meaningfully productive without needing to know all the details or make complicated decisions up front. At the same time, they don't lock you in to only using the "default" options. They're useful for new learners who don't know all the options available or what the "right" choices are, but also for experienced devs who just want a simpler tool without going through all the setup every time. I want Redux Starter Kit to be the same kind of thing, for Redux. Speed Up Getting StartedRedux has a relatively steep learning curve. I don't think anything can ever completely change that. There's a lot of new terms, a bunch of moving pieces, and a different set of expectations that most people aren't used to. We can improve the docs and the teaching experience, but we can't remove all that. However, we can improve the process of actually trying to use Redux, whether it be for the first time or just setting up a new project. I want to give people a way to get working Redux running as fast as possible. For new learners, part of this ties into the Redux docs revamp. I plan to have a page that says "You want to try out Redux? Here's a quick chunk of code. Copy-paste this (or open the CodeSandbox), follow it "blindly" for now, and go through the tutorials next to understand what's actually going on". Similarly, for experienced devs, it should be really easy to add Redux to a React app using Redux Starter Kit. Add one entry to your Simplify Common Use Cases and Remove BoilerplateI want to remove as many potential pain points from the equation as possible:
People use the word "boilerplate" to refer to lots of different things, but mostly it's "writing more repeitive code by hand". The biggest examples are defining action types, writing action creators, and immutable update logic, and to some extent setting up the store. I want the user to be able to do those automatically if they choose to. Handling async request lifecycles falls into this category as well. Most people follow the "action triplet" pattern laid out in the docs, and have thunks that fetch from an API and dispatch actions accordingly. Opinionated Defaults for Best PracticesThe Redux core is deliberately unopinionated, yet we have all kinds of rules that we try to enforce through documentation and "best practices" that we've collectively come up with. Redux Starter Kit should set things up in ways that guide the user towards the right approach, and automatically prevent or detect+warn about common mistakes wherever possible. Specifically:
No Lock-In / Add It IncrementallyIf someone starts an app using Redux Starter Kit, I don't want to prevent them from choosing to do some parts of the code by hand, now or later down the road. Or, for that matter, pulling in other Redux addons of some kind, whether it be middleware or an action/reducer generation library. On the flip side, someone should be able to add Redux Starter Kit to an existing Redux application, and begin using it incrementally, either by adding new logic or replacing existing logic incrementally. They should also be able to pick and choose what pieces they want to use, and not be tied or required to use everything the library offers at once. If someone wants to remove thunks and use sagas instead, or they just want to use Become the "Obvious Default" Way to Use ReduxI still don't know how we're going to juggle teaching the basics of "this is how Redux really works", and "here's what this looks like using Redux Starter Kit". Most of RSK's benefit's only become apparent once you've tried writing some Redux code by hand and started wishing there was a shorter way to do something. But, long-term, I'd like to see RSK become the "obvious default" way for most people to use Redux, in the same way that Create-React-App is the "obvious default" way to start a React project. Sure, there's plenty of other options out there (Gatbsy, NWB), and you can always do it by hand if you want to (Webpack + Babel configs), but it's the choice most people recommend and expect. Don't Solve Every Use Case, and Stay Visibly "Typical Redux"Redux is used in lots of different ways. Because of that, and because the core is so minimal, there's thousands of different Redux addon libraries for a wide spectrum of use cases. We can't solve all those ourselves, and we won't solve all those ourselves. Some specific use cases this library will not address:
Reasonable TypeScript SupportI've recently become aware that apparently more people are using Redux with TypeScript than I had previously thought. Per a recent Twitter poll I ran, looks like it's around 40% or so. I want to support people using TypeScript, same as I want to support anyone else using Redux in specific ways. So, RSK should be reasonably well typed. At the same time, I don't want "perfect typing" to become the enemy of "good functionality". A majority of Redux users still use plain JS, and I don't want to get bogged down writing mile-long arcane type signatures or playing "whack-a-squiggly" when I know the code works right as-is. I'm fine with trying to shape things so they work well in TS, but I'm very willing to ship a feature that has less-than-ideal typing if that's what it takes to get it out the door. (Plus, I still barely know TS anyway, and while I hope to get more TS experience in the very near future, I need to be able to work on this lib myself.) Rationales for InclusionWith all of those thoughts in mind, I'll go through all the existing and proposed functionality, and justify why I'm making these decisions.
|
I'm re-reading the monster "Redux abstractions" thread, and I came across this post from Dan:
Overall, I think what I've laid out meshes fairly well with that list, minus the "built-in memoization" and "indexing and normalization" aspects. |
@denisw I feel that This const increment = createAction('INCREMENT');
const reducer = createReducer( 0, {
[increment]: (state, payload) => state + payload
}) is not much different from const increment = createAction('INCREMENT');
const reducer = createNextState((state, action) => {
switch (action.type) {
case increment.toString() :
return state + action.payload
}
}, 0 ) On the other hand My version of |
A few thoughts regarding slices:
|
@markerikson Thank you so much for putting down your thinking around redux-starter-kit so elaborately. I understand much better now what it should be, why it is how it is, and where it's headed. I would like to writing an answer reflecting on the points you made, and add some more arguments against |
I would also like to express love for the idea of createSlice - it's perfect "starting point" for everyone getting their first exposure to redux: "just use createSlice to make a small counter / todo example to get the feel of it" and then expand on it, or just throw it away and use all the other factory methods as your requirements grow. Also, I'm in favor of including thunks into RSK; Promises - while they could be easier to get the "respond to action" behavior you mentioned - are harder for new developers to understand. We would still need a good API for the actions to support it easilly. I like the idea of tripple-actions for data fetching and I'm using it in all my apps; it's really simple yet powerful and allows you to implement ongoing request monitoring via middlewares / reducers etc. which we rely on a lot. "Allow devTools to be more options instead of boolean" - is there need for that? I'm even questioning if there's need to disable devtools for prod builds. If you know what you're doing, Re @modernserf "there's no way to create namespaced actions" - why not just |
@BTMPL Its not strictly necessary, its just a convenience function to create a bunch of action creators from a single function, without also needing to define a bunch of reducers. Maybe its totally unnecessary in practice? |
Let's move the discussion of exactly how |
hey @markerikson ! const itTestsCreateReducer = (reducers, initState = {}) => {
return testSet => {
testSet.forEach(({ action, mutations }) => {
mutations.forEach(({ props = {}, name, state }) => {
it(`should test ${action} ${name}`, () => {
expect(reducers(state || initState, { type: action, payload: { ...props } })).toMatchSnapshot();
});
});
it('Initial state should match snapshot', () => {
expect(initState).toMatchSnapshot();
});
});
};
}; and it would be used like : // some reducer file
const action = createAction('ACTION');
const initialState = { test: '' };
const reducer = createReducer(initialState, {
[action]: (state, { payload: { value } }) => ({
...state,
test: {
...state.test,
test: value,
},
}),
});
describe('Test reducer', () => {
const testReducer = itTestsCreateReducer(reducer, initialState);
testReducer([
{
action,
mutations: [
{
name: 'Test reducer',
props: {
value: '1',
},
},
],
},
]);
}); Why ? I could create a code sandbox for a live example if you think that this looks good for you ! |
One more idea for the Dev Check Middleware: Warn the user if they dispatch an event as a payload. This easily happen when they hava an action with a default payload and then attach it directly to an onClick handler, not realizing that then the event becomes the payload. |
@phryneas : yeah, that's definitely a footgun, and one I've done a few times myself. I think that the serializability check middleware actually ends up warning/throwing about that one. Also not sure if there's a better way to handle the behavior overall. |
I'm curious what your thoughts are on updating the immer dep to 3.x or eventually 4.x with support for |
Totally fine with that idea. Didn't know newer versions were out. Having said that, you really shouldn't put maps and sets into Redux, and in fact the serializability check middleware that RSK adds by default will yell at you if you do that. |
Yeah, we've run into the serializability check before - I'm not totally clear on why I can put Dates into Redux but not Maps or Sets (but that's probably a discussion for a different forum). |
Can you put Dates into Redux? I mean, I know you can literally, I would have expected that the serializability check in RSK would have flagged those as not serializable. edit I said something here about |
Yep, you're right - the RSK serializability check flags Dates those as not serializable (I was befuddled because we're using Anyway, thank you so much for all your work with RSK - the ergonomics are so nice, it's made me excited about using redux again. |
Just published https://github.com/reduxjs/redux-starter-kit/releases/tag/v0.7.0 , which drops Reselect and slice selectors and adds the ability to customize default middleware. |
I'm generally set on the
That would be released as v0.8.0. Not sure how hard the code changes will be, but the docs will need some significant updates to match:
I can probably make all those changes myself, but I'd definitely appreciate some help. PR #109 has the baseline changes for Any volunteers to help out? My goal would be to push out 0.8.0 within the next 2-3 weeks, and then see if there's any remaining changes from this list I feel are necessary before 1.0. |
I have slacktime tomorrow. I can take a look, but no promises yet, because there's also https://globalclimatestrike.net/ in our city. |
@markerikson I'll be happy to take a look and see if I can help with this. |
Cool. I've created a |
@markerikson I've opened a PR for the code/type changes and the changes directly in the documentation. This still leaves tutorials, examples and the youtube video open. I also have a suggestion for another change: Could we make the reducers available from the slice object? I'd love to have something like const slice = createSlice({
initialState: 1,
name: 'test',
reducers: {
increment(state) {
return state + 1;
},
duplicate(state) {
return state * 2;
},
customLogicAndIncrementAndDuplicate(state) {
const custom = state/3; // some additional logic
return slice.reducers.duplicate(slice.reducers.increment(custom))
}
}
}); And yes, I'm aware that that would be possible by not defining the reducers inline, but then auto inference of the draft type etc. would be lost. |
After further consideration, I've decided to drop the idea of returning the reducer function as the slice result. |
Could we please have the build & tests run for multiple TS versions? Something like the last 4 or 5 minor releases should catch quite a few bugs. |
Sure! Added #216 . |
With 0.8 out the door, I think I'm satisfied for the API for 1.0. Anything else past this should be additive ( I'd like to see if we can get the TSDX build changes in #212 working and switch to that before putting out a couple 1.0 RCs or something. Ideally, I'd like to have 1.0 out the door in the next week (ie, before ReactConf :) ) Any other concerns that need to be addressed? |
almost there... :) |
Sorry if this is covered somewhere and I've missed it. I see support for re-reselect mentioned in this roadmap but didn't see it covered in the API or documentation. Has a solution for re-reselect type of functionality been covered somewhere or did it get dropped? Thanks! |
We dropped Ultimately it was not a good option for TS usage at the time. I could imagine that with today's TS capabilities, a tool like that might actually be doable in a typesafe way. I know I've seen some "typesafe keypath lookup" utils out there. Not sure there's enough real benefit at this point, though. |
Thanks Mark. That makes sense. The typescript tooling in RTK is really impressive. When you say "Not sure there's enough real benefit at this point", do you mean that there just isn't enough demand for a re-reselect feature within the toolkit? |
Oh wait, sorry, I misunderstood the question, and goofed up the change description. What we originally had, and then dropped in 0.7, was https://github.com/planttheidea/selectorator - a Reselect wrapper for writing selectors as string keypaths ( https://github.com/toomuchdesign/re-reselect is a package that also wraps Reselect, but does so to add a keyed item cache to selectors instead of having just a cache size of 1. My mind mixed up the two, sorry! So per the
Improving selector usage for the ecosystem as a whole is definitely something on my radar, but as you can see from that discussion thread, there's been very little follow-up interest from the rest of the community about improving Reselect. I still want to follow-up on that, but I'm juggling entirely too many things myself atm :) |
I'd been thinking about putting up a discussion issue like this already, and someone asked about it today, so figure I might as well.
I deliberately started out at 0.1.0 to give us room to fiddle with the API before reaching stability. The "convert to TS" release put us at 0.4.0. So, what do we feel would be needed to reach a stable 1.0 status?
Here's my rough thoughts on further changes I'm considering:
createSelector
from Reselect,adding a dependency on Re-reselect, and re-exporting itscreateCachedSelector
We might want to have a tiny wrapper around those that only accepts the array form of input selectors, not the arbitrary function arguments formLooking like Reselect v5 may do this as a breaking changecreateSlice
selectFoo
instead ofgetFoo
)?state => state
is pointless. Why are we doing that? What should we do instead?createAction
.type
to the action creators in addition to.toString()
. This helps with JS switch statements, as well as object lookup tables in TS."STARTED/SUCCESS/FAILURE"
action typesredux-promise
I'd like to see if I can make a middleware that checks for side effects that happen in a reducer (at least async requests and uses of randomness)Skipping this ideareduce-reducers
to the toolkitdevTools
to be more options beyond a booleanautodux
to see if there's any other functionality we want to swipeNot saying we have to have all of these, but those are the ideas floating around my head.
Thoughts? Suggestions?
The text was updated successfully, but these errors were encountered: