-
Notifications
You must be signed in to change notification settings - Fork 131
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
Returning actions vs. imperative dispatch #365
Comments
on a lateral note, what's the redux-y way of issuing AJAX requests? I currently have something to the effect of: function fetchData () {
store.dispatch({ type: 'data:fetch:start' })
request('/data.json')
.then((data) => store.dispatch({ type: 'data:fetch:done', data })
.catch((error) => store.dispatch({ type: 'data:fetch:errror', error })
} ...which obviously won't work with the "returning actions" paradigm. |
I do really like that approach. It makes things really simple and pure. The only concerns I had with going that way way:
One really good thing about doing this is that it would be nearly impossible for someone to do something stupid in their component. It pretty much enforces best practice. Could we implement both? It seems like that could be harmless and allow some testing. |
@rstacruz I use redux-effects for composing and managing side-effects in a pure way. For http requests specifically, there is redux-effects-fetch. The primary redux-effects repo is just the thing that orchestrates your side effects, and it can be swapped out with alternatives if you don't like the @11111000000 If you want to see some examples, check out the examples in vdux, a virtual dom library / micro-framework I created that was heavily inspired by deku, virtual-dom, and redux. @anthonyshort Implementing both sounds great to me. At least in the beginning - I think once you want to start growing a reusable component ecosystem it probably makes sense to standardize on one or the other, but for a while I think both would be just fine. As for making things difficult later - I think there is some conceptual trickiness there, however using something like redux-effects or redux-saga, you should be able to compose arbitrarily complex operations that are triggered by the actions you return, and have all that stateful messiness live inside your redux middleware. |
Using redux-effects and redux-effects-fetch you can define your actions declarative without imperative import { bind } from 'redux-effects'
import { fetch } from 'redux-effects-fetch'
import { createAction } from 'redux-actions'
const gotData = createAction('data:fetch:done')
const setError = createAction('data:fetch:error')
const beforeMount = () => {
// returns plain old object
return bind(
fetch('/data.json'),
({value}) => gotData(value),
({value}) => setError(value)
)
}
const reducer = handleActions({
[gotData]: (state, data) => { ...state, data },
[setError]: (state, error) => { ...state, error: error.message }
})
{
type: 'EFFECT_COMPOSE',
payload: {
type: 'FETCH'
payload: { url: '/data.json', method: 'GET' }
},
meta: {
steps: [
[
// success
({value}) => gotData(value),
// failure
({value}) => setError(value)
]
}
}
Well I sort of think the same way. I think sometimes you just can't always synchrounously return actions. I had a discussion with @ashaffer a couple of days ago about const frame = null
const loop = dt => {
const selectedState = select(getState())
if (isEmpty(selectedState)) {
frame ? cancelAnimationFrame(frame) : return
}
// No way I can return here
dispatch(update(
dt,
selectedState,
renderContext
))
frame = requestAnimationFrame(loop)
}
loop() I think the best option is to support both but encourage declarative dispatching over the imperative one. |
Using redux-effects-timeout the return value of your import {raf} from 'redux-effects-timeout'
function handleClick () {
return raf(doSomethingLater)
} The return value of |
I should also add that redux-multi makes this approach much more manageable, by allowing you to return multiple actions as an array, e.g. import {bind} from 'redux-effects'
import {fetch} from 'redux-effects-fetch'
import {createAction} from 'redux-actions'
function render () {
return <button onClick={fetchItems}>Fetch items</button>
}
function fetchItems () {
return [
fetchingItems(),
bind(fetch('/items'), receivedItems)
]
}
const fetchingItems = createAction('fetchingItems')
const receivedItems = createAction('receivedItems') |
The nice thing with the current |
We discussed this briefly in the 2.0 release thread but i'd like to bring it up again so that other people can chime in if they want.
Example:
Imperative dispatch:
Returning actions from events / lifecycle hooks:
The return value of all event handlers and lifecycle hooks is passed to
dispatch
automatically, and your components don't ever even need to know aboutdispatch
.I do understand the perspective that it might create a more difficult learning curve, but IMO deku is already for people looking for the next step up in conceptual purity and simplicity from React, and so should be more tolerant of abstract concepts.
If you give components access to dispatch directly, your event handlers and lifecycle hooks can become stateful, long-lived, and harder to test.
E.g.
Or
These things are hard to reason about and hard to test and right now are the sole source of functional impurity in deku. If you got rid of imperative dispatch, components would be 100% functionally pure / referentially transparent.
This would also make component composition / mixins more powerful, elegant, and idiomatic. If you wanted to proxy the actions emitted by a component in a mixin, right now you'd have to do that by wrapping the component and swapping out dispatch, which is not really the standard definition of composition. If you use return values, you can compose event handlers / lifecycle hooks in a more natural way.
Deku 2.0 is really nice and i'm considering using it instead of virtex for the pending rewrite of our app at my work, but this seems like a critical issue to me.
The text was updated successfully, but these errors were encountered: