Skip to content

MR Notes Vuex store inconsistently namespaces its modules

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Summary

The MR notes Vuex store is composed of a few modules: page, notes and diffs.

The first two do not have namespaced: true set, while diffs does. This means that all the actions, mutations and getters of the first two are added to the global namespace of the root store, while the diffs module's actions, mutations and getters are all namespaced underneath diffs/.

Without namespacing, there's the potential for non-obvious naming collisions between modules (akin to the problem of mixins).

Where this gets particularly confusing, though, is that whether or not a module is namespaced, its state is always nested under the module's name of the root store's state. This is an inconsistency in Vuex, and not anything specific to our usage.

In practice, this means that mapState({ foo: state => state.diffs.foo }) will always work, whether or not the diffs modules is namespaced, but mapState('diffs', ['foo']) will only work if it is namespaced.

This came up in an MR recently. Because the notes module is not namespaced, you cannot directly swap out a mapGetters call (which directly returns a top-level state property) with a mapState call; you must use the callback notation as above.

Improvements

I'd suggest that modules should always be namespaced to clear up this ambiguity.

Risks / Involved components

Several apps load the diffs and notes modules, so many files would need to be updated to correctly bind the modules within components.

Edited by 🤖 GitLab Bot 🤖