Discussion: Scoping via function composition vs. scope as a delimited string or array
jcheroske opened this issue · 18 comments
I can see two ways of tracking the subspacing in the component tree. Both involve altering the redux store provided in the context. The first approach uses function composition, while the second uses a scope string or array.
I think one of the challenges for us, as we discuss these concepts, is finding the language for things. Perhaps we should start another discussion about names?
First approach:
import PropTypes from 'prop-types'
import {Component} from 'react'
class SubspaceProvider extends Component {
static propTypes = {
namespace: PropTypes.string.isRequired
}
static contextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired
}).isRequired
}
static childContextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired
}).isRequired
}
getChildContext () {
const {namespace} = this.props
const {store} = this.context
return {
store: {
...store,
dispatch: (action) =>
store.dispatch({
...action,
type: `${namespace}/${action.type}`
}),
getState: () => store.getState()[namespace]
}
}
}
}
Second approach:
import {get} from 'lodash/fp'
import PropTypes from 'prop-types'
import {Component} from 'react'
class SubspaceProvider extends Component {
static propTypes = {
namespace: PropTypes.string.isRequired
}
static contextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired,
rootStore: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired
}),
scope: PropTypes.array
}).isRequired
}
static childContextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired,
rootStore: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired
}),
scope: PropTypes.array.isRequired
}).isRequired
}
getChildContext () {
const {store} = this.context
const scope = store.scope || []
const rootStore = store.rootStore || store
scope.push(this.props.namespace)
return {
store: {
...store,
dispatch: (action) =>
rootStore.dispatch({
...action,
type: `${scope.join('/')}/${action.type}`
}),
getState: () => get(scope.join('.'))(rootStore.getState()),
rootStore,
scope
}
}
}
}
Each of these approaches works fine for state scoping and action prefixing. I'm curious what pros and cons of each approach people see. I see a clear winner when it comes to supporting middleware, but let's begin the discussion with the more general case.
Discussion continuing from here
(I'm busy at the moment, but I'll try to put some thoughts down tonight)
Got some time... here we go...
For me, when it comes to isolating the sub-store from the root store (which is the primary focus of redux-subspace), the prefixing approach has an incredible advantage over a scope node in that many (most?) middlewares, libraries, and vanilla redux techniques already honour the type field of the action and will include or exclude the action accordingly.
For example given a reducer (from the redux docs):
function todoApp(state = initialState, action) {
switch (action.type) {
case SET_VISIBILITY_FILTER:
return Object.assign({}, state, {
visibilityFilter: action.filter
})
default:
return state
}
}
This will already ignore a namespaced action that has the same type (SET_VISIBILITY_FILTER), as the action type doesn't match. This is just default redux behaviour.
If the scope field approach was used, we would have to ensure that every single reducer in the entire application was written to ensure it also confirmed the scope doesn't match before it accepts or discards the action.
It boils down it being implicit or explicit when dealing with the larger redux eco-system. The prefix approach will be implicitly included/excluded from anything looking at action types, while the scope field approach will need to explicitly included/excluded.
When it comes to getState, the advantage that the mapState approach has over the using the scope field is that you aren't restricted to just the state of the reducer. A common approach we use is to map the application's configuration into the subspace's state, e.g.
mapState = (state, rootState) => ({ ...state.child, config: rootState.config })
I will concede that a common pattern I see (even if the examples and docs or this repo) is:
<SubspaceProvider mapState={(state) => state.child} namespace="child">
...
</SubspaceProvider>
Currently, namespace is optional when creating subspaces but mapState is required. I've been thinking for a while that perhaps mapState should default to (state) => state[namespace] as this is true a lot of the time. The validation would then be that you must provide at least one of them.
If it did that, there actually isn't that much difference then from a consumer's perspective to the scope field approach:
<SubspaceProvider namespace="child1">
<SubspaceProvider namespace="child2">
<SubspaceProvider namespace="child3">
...
</SubspaceProvider>
</SubspaceProvider>
</SubspaceProvider>
...
const child3 = namespaced(..., "child3")
const child2 = namespaced(combineReducers({ child3 }), "child2")
const child1 = namespaced(combineReducers({ child2 }), "child1")
const reducer = combineReducers({ child1 })
An ACTION_TYPE action dispatched from within the child3 subspace will be prefixed as "child1/child2/child3/ACTION_TYPE" and the reducer to handle it will remove all those layers.
Does all this make sense?
Ah, I should have been more clear! The two approaches I described both prefix the action type field. The difference between them is how they track their position in the tree. When dispatch fires on either approach though, the result will be identical prefixed actions.
Then I don't actually follow how your alternative proposition works... Can you run through a basic process flow of an action that is dispatched from two layers deep?
It is important to support the scenario where both global and local state and/or actions are needed in the component or subcomponents – as transparently and hassle-free as possible.
I would rather supply a helper or path (a lens, optimally) for the component to work with than doing things automatically.
Good chance you folks have seen this already, but just in case: the main place for prior discussions regarding encapsulation approaches is https://github.com/slorber/scalable-frontend-with-elm-or-redux . May be useful stuff you can reference there. I also have some related articles in the Redux Architecture#Encapsulation section of my links list.
I haven't dug much further into this particular topic myself beyond cataloging things, but it's a discussion I'm interested in.
@dalgard how does a lens differ from a selector (which is all mapState is), or are they just different terms for the same thing?
@jcheroske was the dispatch in the second approach meant to also be dispatched into the store? I don't see where that is occurring in that example.
For the record, redux-subspace is not quite doing either approach. A simplified version is more like
class SubspaceProvider extends Component {
static propTypes = {
mapState: PropTypes.func.isRequired
namespace: PropTypes.string
}
static contextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired
}).isRequired
}
static childContextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired,
}).isRequired
}
getChildContext () {
const {store} = this.context
let namespace = ''
if (this.props.namespace) {
namespace = `${namespace}/`
}
return {
store: {
...store,
dispatch: (action) =>
store.dispatch({
...action,
type: `${namespace}/${action.type}`
}),
getState: () =>this.props.mapState(store.getState())
}
}
}
}
As I previously mentioned, mapState is required and namespace is currently currently optional, but I would like to make it that if one namespace is provided, mapState is defaults to (state) => state[namespace].
If you add in my proposition in #31, it will look something like:
class SubspaceProvider extends Component {
static propTypes = {
mapState: PropTypes.func
namespace: PropTypes.string
//somehow validate at least one is provided (or a sensible behaviour is neither are?)
}
static contextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired,
namespace: PropTypes.string
}).isRequired
}
static childContextTypes = {
store: PropTypes.shape({
dispatch: PropTypes.function.isRequired,
getState: PropTypes.function.isRequired,
namespace: PropTypes.string
}).isRequired
}
getChildContext () {
const {store} = this.context
let actionNamespace = ""
let storeNamespace = store.namespace || ""
if (this.props.namespace) {
actionNamespace = `${this.props.namespace}/`
storeNamespace = `${storeNamespace}/${this.props.namespace}`
}
let mapState = this.props.mapState || (state) => state[this.props.namespace]
return {
store: {
...store,
dispatch: (action) =>
store.dispatch({
...action,
type: `${actionNamespace}${action.type}`
}),
getState: () => mapState(store.getState()),
namespace: storeNamespace
}
}
}
}
As a side note, does store.getState()[scope.join('.')] actually work in javascript? i.e. would state['a.b.c'] resolve to "123" if state = { a: { b: { c: "123" } } } or must is be state = { 'a.b.c': "123" }?
@jcheroske was the dispatch in the second approach meant to also be dispatched into the store? I don't see where that is occurring in that example.
Note to self: writing code in the airport is questionable at best. Yes, and it was supposed to dispatch off the original store. And the getState was also. I edited the original again.
As a side note, does store.getState()[scope.join('.')] actually work in javascript? i.e. would state['a.b.c'] resolve to "123" if state = { a: { b: { c: "123" } } } or must is be state = { 'a.b.c': "123" }?
Man, I could have sworn I tested that just the other day, just for yucks. But, nope, no dice there. I always use lodash for that kind of thing anyway, and that's what I did just now.
For the record, redux-subspace is not quite doing either approach. A simplified version is more like
I think, imho, that you're doing the first approach pretty much, no? Composing calls to the store as you become more deeply nested vs always calling back to the root. I see you've broken out the state retrieval via a mapper, but the mapper calls the previous level's store. And I see you're passing down the fully qualified scope, but you don't really use it in that code.
@mpeyper A lens doesn't really differ from a selector when only using it to view data (selector = view(myLens) <-- curried). I simply think lenses is a better API in general, if used consistently within a domain.
I'd say the main differences between a "lens" and a "selector" is that (from my very limited knowledge) a lens is generally an abstraction over looking up data from a very specific state path (and also potentially applying updates to that same state path). A selector, on the other hand, normally takes state and could return anything as a result derived from that state input (and possibly other inputs as well).
The reason I asked the question originally has to do with making redux namespacing work with the various side-effect libraries. The way I did it was to remove the scope from the action type as it was passed to the epic, and then apply the scope to any actions emitted. So the epic was listening for global actions, and a wrapper was globalizing incoming actions and re-scoping outgoing actions. It works, but I'm very curious if people are adding and removing scoped sagas, epics or whatever as the corresponding component is mounted and unmounted?
@dalgard @markerikson so selector = (state) => state.my.scope and lens = view('my.scope') (where view would construct a dot path selector for state)? Both would still be given state to resolve the substate, but the lens has less control over how it is applies? Am I on the right track?
@jcheroske middleware that runs outside of the component (like sagas - I don't know enough about epics to say whether the do or not) cause an interesting problem for redux-subspace. Basically, the only action that comes from the component is the one that triggers the saga. After that, all the other actions that come from the saga and go straight back to the global redux dispatch pipeline.
How this was solved for sagas was to essentially create a seperate saga context for the subspaced saga, but it has to be wrapped separately from the component as it doesn't actually originate from within it (you can take a look at my proof of concept for #18 here).
Speaking more generally, wrapping your thing (action, reducer, component, saga, epic, thunk, logic, etc) to remove the namespace on the way in, and attach it on any thing on the way out is my preferred approach. Elevating the thing to global scope will likely come back to bite you in unexpected ways.
Speaking more generally, wrapping your thing (action, reducer, component, saga, epic, thunk, logic, etc) to remove the namespace on the way in, and attach it on any thing on the way out is my preferred approach. Elevating the thing to global scope will likely come back to bite you in unexpected ways.
I guess I didn't mean that the 'thing' was listening to global actions. More that the actions it was listening for were un-scoped, but local. Just like in subspace where a reducer would be written to listen to a global action (widget/RESET) or whatever, and then the reducer would be wrapped so that its now scoped the action's scope would be stripped off on the way in. It looks like you're doing essentially that with sagas as well: wrapping the saga so that the scope is removed on the way in, and then providing a scoped store so that actions dispatched by the saga are scoped on the way out.
This is essentially what I had to do to get epics working, except for the fact that you never call dispatch directly within an epic. An epic is just an observable stream that has actions as input and actions as output. All outputted actions will be dispatched for you. I wrote a wrapper that stripped off the scope of all inputted actions, and put it back on anything coming out.
So I'm assuming you install your sagas at application startup, just like most folks install regular sagas? No fancy install/uninstall?
So I'm assuming you install your sagas at application startup, just like most folks install regular sagas? No fancy install/uninstall?
No, nothing fancy. The top level saga must be wrapped in a store provider to get the root store into the saga's context, but other than that they are treated like normal sagas.
I'd love to see your epics solution. I haven't looked at them at all (they are a concept fro redux-observable right?), so it would be good to see something from someone more familiar with them.
My approach was to include additional info in the action, that would then be used by a small wrapper around an epic-like observable. I added the scope, the un-scoped action type, and a scoped getState(). I don't like the implementation, and it's one of the main reasons I started these discussions. It's also why I started exploring redux-logic, because I've found the observable metaphor difficult in many ways. If you look at ReactiveX/RxJava#2931, you'll see that observables make it hard to pass additional information down the pipeline. You can do it, but you need to use a container object and pass that down the line instead. It means that your epics can't just be written like normal epics, but instead they know that they are part of a fractal component. I don't like that, and so I'm exploring other options.
Here is my enhanceEpic anyways:
import {compact} from 'lodash'
export default epic => (action$, store) => {
const epicContext$ = action$
.map(({globalType: type, scopedGetState: getState, ...rest}) => ({
incomingAction: {
...rest,
type
},
store: {
...store,
getState
}
}))
return epic(epicContext$)
.map((epicContext) => {
if (typeof epicContext.type !== 'undefined') {
return epicContext
}
const {incomingAction, outgoingAction} = epicContext
outgoingAction.type = compact([incomingAction && incomingAction.scope, outgoingAction.type]).join('.')
return outgoingAction
})
}