Vuex: Proposal: Make parent & sibling modules' assets accessible to child modules via local context

[jskrzypek]

What problem does this feature solve?

Simplifies accessing parent/sibling assets by placing relative accessors on child modules' local contexts

Existing API - Accessing child module assets from a parent's local context

For child modules, we do not need to use a path from the root to access their assets, we can just do dispatch('child/childAction') or getters['child/childGetter'] as a shorthand, e.g.:

const parent = {
  actions: {
    bazzifyChild({ state, dispatch, commit, getters }) {
      dispatch('child/changeFoo', 'baz') // child/changeFoo commits a mutation to set child.foo to 'baz'
      // getters['child/foo'] now returns 'baz'
    }
  }
}

Proposed Syntax - '../<type>' or '$parent/<type>'

I want the same thing as the child asset access, except to for assets on the parent module. I think the posix path syntax of '../' is nicely consistent with the current use of the backslash separator: dispatch('../parentAction') commit('../parentMutation') getters['../parentGetter'], and thus similarly for siblings' assets: dispatch('../sibling/siblingAction', payload). The nested state on the local context could access it's parent's state by using state['..'].parentStateVal

An alternative that I present in my proposed API below would be to use '$parent/' instead: dispatch('$parent/parentAction') commit('$parent/parentMutation') getters['$parent/parentGetter'] state.$parent.parentStateVal

Additionally, depending on some benchmarks that I'll try to run next week, it could be that this syntax above is expensive to provide for the getters due to the performance implications of my proposed code changes. If that's the case then an alternative syntax may be to lazily resolve the parent getters similar to what we will do for the state: getters['$parent']['parentGetter'] or getters['..']['parentGetter'].

Motivation

I have an SPA tabs for different entities in my DB with a store module per tab/view. I want to reuse my logic for dealing with paging in the views across all of the views without having to rewrite it. The best way I see to do this is by adding an instance of my paging module as a submodule to each of these view modules.

To make this work, I need the paging child module to be able to access assets on its parent and other children of its parent. I can only get this behavior at the moment by calculating the full path to the parent's or other submodule's assets from the root, which means that my paging module's assets need to know about the submodule's full path.

Currently, to solve this I can use a plugin to dynamically determine which modules need a paging submodule and then call a factory function for each path and then call store.reqisterModule() with each path and created instance of my submodule config.

While this works, this pattern for reusing logic between different modules could become quite cumbersome if I had deeper nesting in my module tree. Moreover, it forces the reusable logic in the submodules to be more stateful (in that they must hold on to their path/namespace). Additionally, because they must be registered in a plugin I cannot use a more static pattern for the submodules and simply import/include them in the modules property of their parent modules without adding more statefulness to the parent modules themselves so they can know their own full module path.

I'll understand if this proposal gets rejected on the grounds that a way to do this exists, but I do believe that this feature might make it significantly easier to create and reuse more complex functionality between submodules.

What does the proposed API look like?

API

Similar to the local context's access to child modules' assets, the api for this feature should give convenient access to a parent's assets.

Here is a simplified example version of the paging submodule and a product parent module that I could use in my app if the proposed feature is adopted:

Function child module – store/modules/paging.js

export default const paging = {
  namespaced: true,
  state: {
    skip: 0,
    limit: 20, // twenty items per page by default
    total: 0 // recieves the total number of items
  },
  getters: {
    skip: state => state.skip,
    limit: state => state.limit,
    total: state => state.total,
    lastPage: (state, getters) => getters.skip + getters.limit >= getters.total
  },
  actions: {
    setPageSize({ commit }, limit) {
      commit('SET_LIMIT', limit)
    },
    next({ state, dispatch, commit, getters }) {
      if (!getters.lastPage) {
        let { skip, limit } = state
        skip += limit
        limit = skip + limit > state.total ? state.total - skip : limit
        dispatch('../find', { ...state.$parent.query, skip, limit })
          .then(() => {
            commit('SET_SKIP', skip)
          })
      }
    },
    previous({ state, dispatch, commit, getters }) {
      if (state.skip !== 0) {
        let { skip, limit } = state
        skip = skip > limit ? skip - limit : 0 // don't page below skip of 0
        dispatch('../find', {  ...state.$parent.query, skip, limit })
          .then(() => {
            commit('SET_SKIP', skip)
          })
      }
    }
  },
  mutations: {
    SET_SKIP(state, skip) {
      state.skip = skip
    },
    SET_LIMIT(state, limit) {
      state.limit = limit
    }
  }
}

Consumer parent module – store/modules/product.js

import paging from './paging'
import { productService } from '@/services'
export default const product = {
  namespaced: true,
  state: {
    products: []
  },
  getters: {
    products: state => state.products,
    query: state => state.query
  },
  actions: {
    find({ state, dispatch, commit }, query) {
      commit('SET_QUERY', query)
      return productService.find(state.query)
        .then(({ data }) => {
          commit('SET_PRODUCTS', data)
        })
    }
  },
  mutations: {
    SET_PRODUCTS(state, products) {
      state.products = products
    },
    SET_QUERY(state, query) {
      state.query = query
    },
  },
  modules: {
    // add the paging submodule
    paging
  }
}

Implementation

The code changes necessary to support this feature are constrained to the makeLocalContext() function in src/store.js as well as makeLocalGetters() and maybe getNestedState().

However, supporting it for the different assets carry different computational complexities/overhead, because of how the localContext is constructed.

Actions/Mutations (via the context's dispatch()/commit())

To support relative paths/namespaces for dispatches and commits is relatively simple, we just need to parse any ../ namespace components at the start of the type passed to dispatch()/commit(). We can do this by altering slightly. This introduces no computational overhead if the type passed to the dispatch()/commit() is a normal local type, and very little additional overhead if the type does start with some number of ../ components.

 /**
  * make localized dispatch, commit, getters and state
  * if there is no namespace, just use root ones
  */
 function makeLocalContext (store, namespace, path) {
   const noNamespace = namespace === ''

   const local = {
     dispatch: noNamespace ? store.dispatch : (_type, _payload, _options) => {
       const args = unifyObjectStyle(_type, _payload, _options)
       const { payload, options } = args
       let { type } = args

       if (!options || !options.root) {
-        type = namespace + type
+        let typeNamespace = namespace
+        if (type.startsWith('../')) {
+          let sliceEnd = 0
+          while (type.startsWith('../'))
+            type = type.slice(3) // remove the '../' from the type
+            sliceEnd--           // move the end of the slice one item back
+          }
+          typeNamespace = store._modules.getNamespace(path.slice(0, sliceEnd))
+        }
+        type = typeNamespace + type
         if (process.env.NODE_ENV !== 'production' && !store._actions[type]) {
           console.error(`[vuex] unknown local action type: ${args.type}, global type: ${type}`)
           return
         }
       }

       return store.dispatch(type, payload)
     },

     commit: noNamespace ? store.commit : (_type, _payload, _options) => {
       const args = unifyObjectStyle(_type, _payload, _options)
       const { payload, options } = args
       let { type } = args

       if (!options || !options.root) {
-        type = namespace + type
+        let typeNamespace = namespace
+        if (type.startsWith('../')) {
+          let sliceEnd = 0
+          while (type.startsWith('../'))
+            type = type.slice(3) // remove the '../' from the type
+            sliceEnd--           // move the end of the slice one item back
+          }
+          typeNamespace = store._modules.getNamespace(path.slice(0, sliceEnd))
+        }
+        type = typeNamespace + type
         if (process.env.NODE_ENV !== 'production' && !store._mutations[type]) {
           console.error(`[vuex] unknown local mutation type: ${args.type}, global type: ${type}`)
           return
         }
       }

       store.commit(type, payload, options)
     }
   }

Getters

Unlike actions & mutations, I don't see a way to make only a small change to the current mechanism for makeLocalGetters() that only adds computational complexity in the case that the consumer uses the ../ components in the getter's type at the time that they try to access the parent getters/state. I have thought of two approached that trade off in how cumbersome they would be to implement vs consume this behavior.

Getters – Straightforward (naïve?) approach

The most straightforward approach is to adapt the current makeLocalGetters() behavior to do something similar to my proposed change for dispatch()/commit(). I.e. it would add an accessor to gettersProxy for every getter in the store, simply calculating the relative '../' syntax paths for the current namespace (when you allow upwards traversal, any leaf in the tree is accessible from any node). To calculate the relative getter paths, we can adapt node's path.relative() for posix systems (see my gist here),
and then use it as a helper function in this change to makeLocalGetters():

 function makeLocalGetters (store, namespace) {
   const gettersProxy = {}

   Object.keys(store.getters).forEach(type => {
-    // skip if the target getter is not match this namespace
-    if (type.slice(0, splitPos) !== namespace) return
-
-    // extract local getter type
-    const localType = type.slice(splitPos)
+    // extract local getter type
+    const localType = getNamespaceRelativeType(namespace, type)

     // Add a port to the getters proxy.
     // Define as getter property because
     // we do not want to evaluate the getters in this time.
     Object.defineProperty(gettersProxy, localType, {
       get: () => store.getters[type],
       enumerable: true
     })
   })

   return gettersProxy
 }

However, this approach may have performance issues (I'm not sure yet). As I understand it, the computational footprint of the current implementation of makeLocalGetters() seems dominated by how many getter accessors need to be added to the gettersProxy using Object.defineProperty(). The performance of this process may vary significantly based on the JVM used by the browser (or node/io.js for SSR). Moreover, because makeLocalGetters() is used as the accessor function to lazily get all of the getters available to the local context, it gets called quite a lot during normal app operations. This means that, with my proposed change, any performance implications of makeLocalGetters() would grow with the number of getters in the entire store, as opposed to just the getters available below the current namespace. On the other hand, it could be that using Object.defineProperty() with only a lazily evaluated get: () => store.getters[type] is fast and cheap in modern browsers next to render cycles, so making this code change would be straightforward.

It seems too naïve to assume one way or the other, so I'm going to some realistic, app-scale benchmarks to see what effects this change would actually have. Any advice/help/input would be more than welcome.

Getters – Lazily evaluated parent-local getters approach

Another option I've considered is that we could add an additional accessor to the getterProxy with a name like $parent or '..' property to the getters object that lazily returns the local getters for the parent module.

This change could be done in relatively few lines of code:

-function makeLocalGetters (store, namespace) {
+function makeLocalGetters (store, namespace, path) {
   const gettersProxy = {}

   const splitPos = namespace.length
   Object.keys(store.getters).forEach(type => {
     // skip if the target getter is not match this namespace
     if (type.slice(0, splitPos) !== namespace) return

     // extract local getter type
     const localType = type.slice(splitPos)

     // Add a port to the getters proxy.
     // Define as getter property because
     // we do not want to evaluate the getters in this time.
     Object.defineProperty(gettersProxy, localType, {
       get: () => store.getters[type],
       enumerable: true
     })
   })

+  let parentPath = path.slice(0, -1)
+  let parentNamespace = store._modules.getNamespace(parentPath)
+
+  Object.defineProperty(gettersProxy, '..', {         // or using '$parent'
+    get: () => parentNamespace === ''
+      ? store.getters
+      : makeLocalGetters(store, parentNamespace, parentPath),
+    enumerable: true
+  })
+
   return gettersProxy
 }

This would then make it possible to access getters up the chain lazily, and potentially reduce the calls to makeLocalGetters(). It could be used with syntax like that below. N.B. that this syntax is a bit counter to the typical getters intuition of the getters being a flattened hash of all of the getters.

  // usage in a child module's getters
  const getters = {
    parentVal: (state, getters) => getters['..'].val,
    grandParentVal: (state, getters) => getters['..']['..'].val,
    siblingVal: (state, getters) => getters['..']['sibling/val']

    // or if using '$parent'

    parentVal: (state, getters) => getters.$parent.val,
    grandParentVal: (state, getters) => getters.$parent.$parent.val,
    siblingVal: (state, getters) => getters.$parent['sibling/val']
  }

  // usage in a child module's actions
  const actions = {
    siblingActionWithParentGetter({ state, getters, dispatch }, targetVal) {
      if (getters['..'].val !== targetVal)
        return dispatch('../sibling/getNewParentVal')

      // or if using '$parent'

      if (getters.$parent.val !== targetVal)
        return dispatch('../sibling/getNewParentVal')
    }
  }

State

Similar to the getters, we resolve the nested state at the time that it is accessed on local, but before we know which specific type on the state will be accessed, so we cannot follow the approach we used for dispatch()/commit(). Unlike the getters, the state is a normal(ish) JSON structure, not a flattened lookup hash, so it is more natural for the parent states to be accessed via a property, giving us the option of just adding an accessor to each level of the nested state as we traverse downwards in getNestedState(). This might not be the best approach, but using Object.create() should leave each state[key] nested state unmodified while allowing us to add the accessor property for the parent.

 function getNestedState (state, path) {
   return path.length
-    ? path.reduce((state, key) => state[key], state)
+    ? path.reduce((state, key) => Object.create(state[key], {
+        $parent: {            // alternately, '..'
+          get: () => state,
+          enumerable: true
+        }
+      }), state)
     : state
 }

Again, as with the getters, this function runs whenever the local state is accessed from the context, which is a common operation (maybe even moreso than the getters), so any performance implications of this change could be very significant.

[ktsn]

Thanks for the detailed proposal. However, this feature is a bit too complicated to me...

At first, the module structure and the namespace structure is not always consistent, then this API would be confusable in such case. For example, in the following code, what value should state.$parent.value return if we refer it in the module baz? To consistent with getters/actions/mutations, I think it should return foo's state since it is in a parent namespace but it would not be intuitive.

const foo = {
  namespaced: true,
  state: { value: 1 }

  modules: {
    bar: {
      state: { value: 2 }

      modules: {
        baz: {
          namespaced: true,
          state: { value: 3 },

          actions: {
            test ({ state }) {
              state.$parent.value // Which value does this expression return?
            }
          }
        }
      }
    }
  }
}

To be simpler, I would prefer to define all related modules under the same namespace to let a module reusable like below. In this example, the module foo and bar is separated from the root namespace automatically but they can refer each other in short syntax. To deal with such use case, namespaced is not strongly coupled with the module structure.

const parent = {
  namespaced: true,

  modules: {
    foo: {
      actions: {
        fooAction({ dispatch }) {
          dispatch('barAction')
        }
      }
    },

    bar: {
      actions: {
        barAction(ctx) {
          // ...
        }
      }
    }
  }
}

Of course, the naming of getters/mutations/actions in foo and bar may conflict since they are in the same namespace but I think we should be able to handle them by ourselves if they are really a relevant module group.

[jskrzypek]

Those are good points.

Regarding the first, yes, I forgot to consider that the module structure and the namespaces are not always consistent. However, in the case where they are not consistent, accessing child/nested states uses the path/module structure, while accessing getters/actions/mutations uses the namespace. As I understand it, this is already a place where intuition breaks down a bit in Vuex's current behavior. To keep the reasoning consistent the feature should probably follow the same split: state uses the path to resolve the parent, while getters/actions/mutations use the namespace.

This way state values are always resolvable. The getters may possibly be confusing in situations like this one, but this seems like a non-issue since any getter name collisions would simply not set getters coming from the getters and vuex will show an error in the console. For this reason I think that having the reusable modules always be namespaced makes more sense than not.

Regarding your second point, that may not a bad idea for some things... I think I need to progress some with my own use of my paging and fetching submodules and then I should have a more mature use case and example to show.

[kiaking]

Closing due to inactivity. As ktsn mentioned, it might be too complicated to handle each edge cases. I think it would be easier to have only local context and root context with the option of root: true.