Closed gaearon closed 9 years ago
Hmm, why would the state live on the Redux instance? Didn't we decide that the true state "lives" inside the dispatcher?
Hmm, why would the state live on the Redux instance?
It's currently inside this.state
on Redux instance so that anyone can dehydrate it (getState()
) and so components can ask the current state. Indeed, it's just a copy of the real state held inside the dispatcher.
The dispatcher is given setState
callback precisely to set that state field (and let the world know it has changed).
Indeed, it's just a copy of the real state held inside the dispatcher.
Exactly, so if the true state lives inside the dispatcher, why does meta state need to leak out to the Redux instance at all? Can't it be completely self-contained?
Dispatcher needs to be completely replaceable at any point of time for hot reloading. If it doesn't fully yield its state to the Redux instance “for keeps”, the next dispatcher won't receive any hidden fields on hot reload.
Okay, that makes sense. But in that case, I think the dispatcher should be completely stateless, and we pass it a getState()
function rather than having a separate copy of the state atom, which is confusing and bug prone.
So then the state would live completely on the Redux instance and nowhere else.
To summarize what I'm proposing:
createRedux({
dispatcher: (getState, setState, dispatch) => {...} // returns a pure dispatch function, no state
prepareState: (rawState) => {...} // returns state prepared for consumer, stripped of "meta"
})
Ignore the particulars of the API and just focus on the inputs and outputs
I was worried getState
might give the impression that the state might somehow change outside without dispatcher's knowledge which isn't true. But perhaps you're right this API makes more sense.
What is the dispatch
argument for? Middleware?
I thought you were proposing (getState, setState) => (action) => ()
.
I thought you were proposing (getState, setState) => (action) => ().
I am, just put dispatch()
in there as well for, yes, middleware. Should bring it up in a separate issue, but the problem right now is (to use the default middleware as an example) there's no way to do a truly recursive dispatch:
export default function thunkMiddleware(getState) {
return (next) => {
const recurse = (action) =>
typeof action === 'function' ?
action(recurse, getState) :
next(action);
return recurse;
};
}
recurse()
will recursively call the current middleware, but not any that were executed before it in the chain.
Can you PR your proposed changes when you get some time?
Sure, I will tonight
Thank you! I really like where this is going :tada:
We might not need a dispatcher. Here's a variation on “Redux inception” that I think might be the way to go.
Consider this variation of Redux
:
export default class Redux {
constructor(reducer, initialState) {
this.state = initialState;
this.listeners = [];
this.replaceReducer(reducer);
}
getReducer() {
return this.reducer;
}
replaceReducer(nextReducer) {
this.reducer = nextReducer;
this.dispatch({ type: '@@INIT' });
}
dispatch(action) {
const { reducer } = this;
this.state = reducer(this.state, action);
this.listeners.forEach(listener => listener());
}
getState() {
return this.state;
}
subscribe(listener) {
const { listeners } = this;
listeners.push(listener);
return function unsubscribe() {
const index = listeners.indexOf(listener);
listeners.splice(index, 1);
};
}
}
Very little here, right?
Then consider this:
import timeTravel, { ActionTypes, StateKeys } from './timeTravel';
import { createRedux } from '../../../src/index';
export default function createDebugRedux(reducer) {
const timeMachine = createRedux(timeTravel(reducer));
return {
timeMachine: timeMachine,
subscribe: ::timeMachine.subscribe,
getReducer: ::timeMachine.getReducer,
replaceReducer: ::timeMachine.replaceReducer,
dispatch(action) {
timeMachine.dispatch({
type: ActionTypes.PERFORM_ACTION,
action
});
},
getState() {
return timeMachine.getState()[StateKeys.CURRENT_STATE]
}
};
}
In this example, timeTravel()
is a higher-order reducer. It takes your existing reducer that accepts your app's actions and manages your app's state. It returns a fancy reducer that accepts actions like COMMIT
, ROLLBACK
, JUMP_TO_STATE
, and, in addition, PERFORM_ACTION
that corresponds to the user trying to perform an action in the app. The real action is a field on this meta-action. The fancy reducer returned by timeTravel()
also manages its own state. It needs to remember the PENDING_ACTIONS
, SAVED_STATES
, etc. But it also manages a field CURRENT_STATE
corresponding to whatever the app should render.
Our createDebugRedux
creates a “real” Redux instance with the higher-order time travely reducer, but it lifts the actions by wrapping them in PERFORM_ACTION
meta-actions and unlifts the state by unwrapping the CURRENT_STATE
property from the meta-state. Because of this, it looks exactly as the normal Redux instance to the app, but it is smarter inside. All the other methods are proxied because their subscriptions and the underlying reducer are shared. In production, you'd just use createRedux
as is.
Why do it like this? Using Redux to implement Redux devtools is convenient, but that's not the only reason. Notice that we also export the real Redux instance as a field on the wrapper.
This lets us connect devtools like this:
const reducer = composeReducers(reducers);
const redux = createDebugRedux(reducer);
export default class App {
render() {
return (
<div>
<Provider redux={redux}> // <------- fake Redux instance
{() => <TodoApp /> }
</Provider>
<Provider redux={redux.timeMachine}> // <------ time machiney actions and state!
{() =>
<DebugPanel top left bottom>
<ReduxMonitor />
</DebugPanel>
}
</Provider>
</div>
);
}
}
Guess what ReduxMonitor
does. It's a regular React component that connects to the real Redux instance via Connector
. Because it connects to the underlying time traveley instance, it can read time travely state (e.g. PENDING_ACTIONS
and SAVED_STATES
) and fire time travely actions (e.g. JUMP_TO_STATE
). Nice huh?
One thing I don't mention here is initialState
. I'm not sure how to approach it best yet, but I'm sure there's a nice way to fit it in.
Finally, the middleware. I don't have it here yet, but it seems to be that the middleware is just (Redux) -> (Redux)
. (Not the actual type; just the signature.) In other words, the middleware does not have to be built into the core Redux
code. The composeMiddleware
function can just be a helper to wrap Redux instance's dispatch
methods while proxying all other methods as is.
@acdlite I'd love your input on this. If you agree it's the way to go this probably supersedes your current PRs. The “stateless dispatcher” won't matter because there's no dispatcher now, and the other PR will need to be updated because we can try to implement composeMiddleware
as (Redux, middleware) -> Redux
if that makes sense.
:+1: Yes, I like this
Sounds awesome! Just have one question though (perhaps I didn't understand correctly):
Since you are wrapping normal action dispatches inside another meta action, what would happen for action creators that return thunk actions? Suppose:
export function loadStuff() {
return dispatch => {
api.getStuff().then(stuff => dispatch({type: 'STUFF_RECEIVED', stuff}));
};
}
Then if I'm using time travel, I'll be wrapping the thunk inside the meta action PERFORM_ACTION
, and thus whenever I want to re-run this action (because I JUMP_TO_STATE
or whatever), then I'll be doing again the api call, which is a side effect we don't want to cause.
Is that something you already thought of and perhaps I didn't understand? I guess that's the part you were talking in the end about middleware, but didn't quite get it.
@leoasis Yes, this is why I am against re-implementing the existing action middleware as this new API — which is great, but shouldn't be the sole extension point. Action middleware is about transforming a stream of raw actions (which could be functions, promises, observables, etc.) into proper action payloads that are ready to be reduced.
Ah I see it now, so you'll be keeping both "middlewares" (you @acdlite clarified this to me even further here https://github.com/gaearon/redux/pull/166#issuecomment-114201529). I'll give it more thought then.
I'm probably missing something, but for this:
export default function createDebugRedux(reducer) {
const timeMachine = createRedux(timeTravel(reducer));
return {
timeMachine: timeMachine,
subscribe: ::timeMachine.subscribe,
getReducer: ::timeMachine.getReducer,
replaceReducer: ::timeMachine.replaceReducer,
dispatch(action) {
timeMachine.dispatch({
type: ActionTypes.PERFORM_ACTION,
action
});
},
getState() {
return timeMachine.getState()[StateKeys.CURRENT_STATE]
}
};
}
Wouldn't you need to implement your own replaceReducer
to rewrap the new one in timeTravel
, and also save the inner reducer to return in getReducer
? Otherwise you'll break the time travel if you replace the reducer and leak it out of you get the reducer.
So something like:
export default function createDebugRedux(reducer) {
const timeMachine = createRedux(timeTravel(reducer));
return {
timeMachine: timeMachine,
subscribe: ::timeMachine.subscribe,
getReducer() { return reducer; },
replaceReducer(newReducer) {
reducer = newReducer;
timeMachine.replaceReducer(timeTravel(newReducer));
},
dispatch(action) {
timeMachine.dispatch({
type: ActionTypes.PERFORM_ACTION,
action
});
},
getState() {
return timeMachine.getState()[StateKeys.CURRENT_STATE]
}
};
}
@bdowning Scratch that, I see it now. Yeah I think you're right.replaceReducer()
exists to support hot reloading. We can assume that timeMachine
's reducer is the same across hot reloads.
You seem right. I wonder how it worked for me last time I was testing :-O
@bdowning
It seems that it works either way because, strictly speaking, the only requirement is that currentRedux.replaceReducer(nextRedux.getReducer())
works. Even if the wrapper Redux's getReducer
/replaceReducer
is proxied as is, this will still work.
Perhaps there's a better API possible here (redux.accept(nextRedux)
?) that makes this more obvious.
Some updates in terms of how we think about it. https://github.com/gaearon/redux/issues/113#issuecomment-114049804 is still very relevant, but I'm thinking of slightly different API.
We've got some basic new terminology:
// not super strict but shows the idea
// Stuff you write
Reducer: (State, Action) -> State
// Stuff Redux provides
Store: {
dispatch: Action -> (),
subscribe: Function -> Function -> (),
getState: () -> State
}
// Turns stuff you write into stuff Redux provides
createStore: Reducer -> Store
Here's how time travel fits here:
alias TimeTravelReducer = Reducer
alias TimeTravelStore = Store
// turns your reducer into reducer that holds time travel state and handles time travel actions
lift: Reducer -> TimeTravelReducer
// turns time travel store into a normal store that looks like your store to the app
unlift: TimeTravelStore -> Store
// wraps lift and unlift
timeTravel: createStore -> createStore
import timeTravel from 'redux-time-travel';
const createTimeTravelStore = timeTravel(createStore);
const store = createTimeTravelStore(reducer);
// equivalent to
import { lift, unlift } from 'redux-time-travel';
const store = unlift(createStore(lift(reducer));
I'm not sure how coherent it is at this point.. It'll be easier when you see the code (soon :-).
Is this time travel tool going to be in core, or is the thrust of this issue here just figuring out how to rearrange the core API to allow things like time travel to be easily written (i.e. by Store [new terminology] composition)?
It's going to be outside the core. Yes, this issue is just about making this possible (and easy).
And lift
is indeed a transducer here. See #176
So here is why I built Redux: https://www.youtube.com/watch?v=xsSnOQynTHs
Redux DevTools will appear here in a week or so: https://github.com/gaearon/redux-devtools Watch the repo for updates!
redux DevTools.
glad to watch this.
redux had been plan to our production release, it's best innovation for flux.
Redux DevTools 0.1.0 are released. https://github.com/gaearon/redux-devtools
Reviving musings from #6, I'm going to post my progress on working on time travel & other devtool goodness here.
I have a working proof of concept of time travel that uses the default dispatcher and relies on store composition to keep a log of state. This is not feature-complete (only “jumping to state #N” is supported), but shows how to implement such functionality with Store composition:
Usage:
This relies on the root atom being “normal” a JS object though. I think we'll need to store it internally as
atom
field instead inside a plain JS object created by us to support such “hidden” tool state.I'd accept a PR that hides the user store's state inside an
atom
field on the Redux instance'sstate
object, at the same time preserving the current API and behavior exactly.Redux.getState()
should keep retuning that atom. Any other fields on the internalstate
will be assumed to be only used by the devtools.