Skip to content
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

Remove dispatcher (WIP) #166

Closed
wants to merge 1 commit into from
Closed

Remove dispatcher (WIP) #166

wants to merge 1 commit into from

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jun 22, 2015

This begins to simplify the API to allow #113 (comment). It simply removes the dispatcher because it turns out to be unneeded. This also makes createRedux(reducer: Object | Function) make more sense as the shortcut.

I think this supersedes #119.

What's lacking:

  • Support for dispatch-level middleware
  • Tests
  • Updated examples

Ideally I'd like to implement middleware API proposed in #120, but not inside Redux—instead, we'd call compose inside createRedux helper. I think that divides their responsibilities better. Technically, I'd prefer the “new” dispatch middleware to be implemented by wrapping the dispatch function and returning something like { ...redux, dispatch: composeMiddleware(redux.dispatch, ...middlewares) } from createDispatch(reducer, ...middlewares). This would match the wrapping approach suggested here for implementing time travel, but on a smaller (and much more restricted) scale.

If somebody would like to re-implement dispatch middleware like I suggested above in this branch, please feel free to!

cc @acdlite

@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

I really, really like this approach.

The only objection that comes to mind regards terminology. I think we need distinct terms for "dispatch-level middleware" (I've called it "action middleware" elsewhere) and the "Redux-level middleware" you describe in #113 (comment). Although conceptually they are both middleware, it would be confusing to call them both by such similar names.

Also, since we're going to rename the Redux class to Store, the API will actually be createDebugStore(). Suggestion: call this a higher-order store.

@leoasis leoasis mentioned this pull request Jun 22, 2015
@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

Oh and just to be clear, I do think we should have separate concepts for dispatch-level "middleware" and store-level "middleware". While it's true that dispatch-level middleware can be implemented as store-level middleware, the concerns are different. Dispatch-level middleware is concerned only with transforming a stream of actions, whereas store-level middleware is concerned with transforming both the action stream and the stream of state atoms.

Also, dispatch-level middleware really needs to occur first: before it is sent through a chain of composed reduxes/stores. See the point raised by @leoasis #113 (comment).

@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

@gaearon Okay, haha, now I get what you meant by this:

Technically, I'd prefer the “new” dispatch middleware to be implemented by wrapping the dispatch function and returning something like { ...redux, dispatch: composeMiddleware(redux.dispatch, ...middlewares) } from createDispatch(reducer, ...middlewares)

So then I think we're in agreement! This should be a helper called something like applyMiddleware(), which is really just a higher-order store*:

function applyMiddleware(store, middlewares) {
  // Dispatch actually comes last, not first
  return {...store, dispatch: composeMiddleware(...middlewares, store.dispatch) }
}

This should always be the last higher-order store before giving it the Provider, that way the action stream is transformed before it gets to time machine et al.

const store = applyMiddleware(createDebugStore(reducer), middlewares);
const timeMachine = store.timeMachine;

<Provider store={store} />
<Provider store={timeMachine} />

*Can we please call it that? The more I say it the more I like it.

@gaearon
Copy link
Contributor Author

gaearon commented Jun 22, 2015

Yeah that's what I wanted!

@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

@gaearon What do you think of the term "higher-order store" and using "middleware" exclusively for action middleware?

@acdlite acdlite mentioned this pull request Jun 22, 2015
@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

@gaearon Suggestion: higher-order stores should accept a store, not the reducer. So createDebuggerStore(reducer) should actually be createDebuggerStore(store):

// This
const store = createDebuggerStore(createStore(reducer));

// instead of this
const store = createDebuggerStore(reducer);

That way they can be composed. (In fact, they can be composed using the same function we use to compose middleware.)

@gaearon
Copy link
Contributor Author

gaearon commented Jun 22, 2015

I don't like composing Stores because I want there to be only a single source of truth in the app. Either a “simple” Store (production), or a “powerful” Store that the time travely thing “dumbs down” so it looks like “simple” store to the app (development).

If there are two Stores, there is no single source of truth, is there? Seems like unnecessary overhead to me. Also, we'd need to extract the inner Store's reducer anyway because that's the only thing from it that the “powerful” Store actually cares about.

@acdlite
Copy link
Collaborator

acdlite commented Jun 22, 2015

There aren't two stores, there's a single store and multiple proxies (or higher-order stores). The internals are the same as your proposal, just a slightly different API.

@taylorhakes taylorhakes mentioned this pull request Jun 23, 2015
@gaearon
Copy link
Contributor Author

gaearon commented Jun 23, 2015

In the end we seem to have converged with @acdlite on the time travelling signature being createRedux -> createRedux. This is because it needs to both “lift” the reducer and “unlift” the returned Redux instance. (And yes, we need the terminology glossary badly at this point :-)

@gaearon gaearon mentioned this pull request Jun 23, 2015
13 tasks
@gaearon
Copy link
Contributor Author

gaearon commented Jun 23, 2015

I'd appreciate somebody helping out here. I'm still busy with my talk so I'm unlikely to finish this before July begins.

gaearon added a commit that referenced this pull request Jun 30, 2015
* “Stateless Stores” are now called reducers. (#137 (comment))
* The “Redux instance” is now called “The Store”. (#137 (comment))
* The dispatcher is removed completely. (#166 (comment))

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: #113 (comment).
* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that Actions have to be plain object. Use middleware for transforming anything else into the actions.

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
gaearon added a commit that referenced this pull request Jun 30, 2015
Naming:

* “Stateless Stores” are now called reducers. (#137 (comment))
* The “Redux instance” is now called “The Store”. (#137 (comment))
* The dispatcher is removed completely. (#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: #113 (comment).
* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that Actions have to be plain object. Use middleware for transforming anything else into the actions.

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
gaearon added a commit that referenced this pull request Jun 30, 2015
Naming:

* “Stateless Stores” are now called reducers. (#137 (comment))
* The “Redux instance” is now called “The Store”. (#137 (comment))
* The dispatcher is removed completely. (#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: #113 (comment).
* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
@gaearon
Copy link
Contributor Author

gaearon commented Jun 30, 2015

Superseded by #195.

@gaearon gaearon closed this Jun 30, 2015
@gaearon gaearon deleted the no-dispatcher branch June 30, 2015 21:32
gaearon added a commit that referenced this pull request Jun 30, 2015
Naming:

* “Stateless Stores” are now called reducers. (#137 (comment))
* The “Redux instance” is now called “The Store”. (#137 (comment))
* The dispatcher is removed completely. (#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: #113 (comment).

Correctness changes:

* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.
* Nested dispatches are now handled gracefully. (#110)

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
gaearon added a commit that referenced this pull request Jun 30, 2015
Naming:

* “Stateless Stores” are now called reducers. (#137 (comment))
* The “Redux instance” is now called “The Store”. (#137 (comment))
* The dispatcher is removed completely. (#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: #113 (comment).

Correctness changes:

* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.
* Nested dispatches are now handled gracefully. (#110)

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
pandafulmanda pushed a commit to pandafulmanda/testing-example that referenced this pull request Aug 2, 2018
Naming:

* “Stateless Stores” are now called reducers. (reduxjs/redux#137 (comment))
* The “Redux instance” is now called “The Store”. (reduxjs/redux#137 (comment))
* The dispatcher is removed completely. (reduxjs/redux#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: reduxjs/redux#113 (comment).

Correctness changes:

* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.
* Nested dispatches are now handled gracefully. (reduxjs/redux#110)

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
timdorr pushed a commit to reduxjs/examples that referenced this pull request Sep 10, 2019
Naming:

* “Stateless Stores” are now called reducers. (reduxjs/redux#137 (comment))
* The “Redux instance” is now called “The Store”. (reduxjs/redux#137 (comment))
* The dispatcher is removed completely. (reduxjs/redux#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: reduxjs/redux#113 (comment).

Correctness changes:

* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.
* Nested dispatches are now handled gracefully. (reduxjs/redux#110)

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
lovelypuppy0607 added a commit to lovelypuppy0607/redux that referenced this pull request May 11, 2023
Naming:

* “Stateless Stores” are now called reducers. (reduxjs/redux#137 (comment))
* The “Redux instance” is now called “The Store”. (reduxjs/redux#137 (comment))
* The dispatcher is removed completely. (reduxjs/redux#166 (comment))

API changes:

* <s>`composeStores`</s> is now `composeReducers`.
* <s>`createDispatcher`</s> is gone.
* <s>`createRedux`</s> is now `createStore`.
* `<Provider>` now accepts `store` prop instead of <s>`redux`</s>.
* The new `createStore` signature is `createStore(reducer: Function | Object, initialState: any, middlewares: Array | ({ getState, dispatch }) => Array)`.
* If the first argument to `createStore` is an object, `composeReducers` is automatically applied to it.
* The “smart” middleware signature changed. It now accepts an object instead of a single `getState` function. The `dispatch` function lets you “recurse” the middleware chain and is useful for async: reduxjs/redux#113 (comment).

Correctness changes:

* The `dispatch` provided by the default thunk middleware now walks the whole middleware chain.
* It is enforced now that raw Actions at the end of the middleware chain have to be plain objects.
* Nested dispatches are now handled gracefully. (reduxjs/redux#110)

Internal changes:

* The object in React context is renamed from <s>`redux`</s> to `store`.
* Some tests are rewritten for clarity, focus and edge cases.
* Redux in examples is now aliased to the source code for easier work on master.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants