Refactor rootStore.state.config to dedicated module

[pkarw]

What is the motivation for adding / enhancing this feature?

The rootStore.state.config.entities.attribute.includeFields kind of paths are all around the code. We should create a dedicated module ConfigManager (like RouteManager) to keep the config.

Note: The thing is that the config needs to be updated runtime

What are the acceptance criteria

• the old pattern rootStore.state.config should still work for backward compatibility
• all the instances in the core + theme of rootStore.state.config replaced by this new fancy module

[filrak]

Imho it shouldn't be a module. More like a core lib

[pkarw]

Yes, core/lib

[pkarw]

What would You say for sth like:

  import { currentConfig } from '@vue-storefront/core/lib/config'
  if (currentConfig().images.useExactUrlsNoProxy) {

instead of:

  import rootStore from '@vue-storefront/core/store'
  if (rootStore.state.config.images.useExactUrlsNoProxy) {

?

[filrak]

why currentConfig? imho it should be a single entity (config) and thats all. Also - why it's a function? It's not so good for performance to return such a big object each time we access it

[pkarw]

This is a function because we'll be taking the config from rootStore.state.config as it was done before. It's by purpose (to have the option to dynamically reload the config with the window.__INITIAL_STATE__) we just don't want to have the rootStore.state references all over the code. So the function is cool and the naming was like currentStoreView() we have defined in the multistore.ts

[filrak]

But we can easily modify plain JS objects too. What is a purpose of making this huge JSON reactive and part of VS state instead of having it in a separate module as an Object?

Imho it's a huge overcomplication

[filrak]

If the purpose of this issue is just to make alias for current behavior it doesn't make sense at all. Having config in a Vuex is a terrible idea that is really bad for either runtime and loading performance. It's not even application state.

[pkarw]

https://github.com/DivanteLtd/vue-storefront/pull/1800/files

We're using the state to dynamically reload the config if this mode is enabled. Not sure if it could anyway lower the performance - we do have much larger objects in state (say: product's list) :)

[filrak]

This doesn't answer my question why we are not using plain object for config instead of using Vuex state.

Despite all the technical reasons why it shouldn't be part of apps state - the config itself with so many bindings in Vuex can make VS lag on low-end devices.

[pkarw]

Filip, the dynamicReload is by default off (low-end devices) - moreover by using state filters (included in #1800) You can pass just the things You need to override with the state - not the full config. This is in fact optimal, as the second option for dynamicConfigReload would be to have separate request which really will block the page rendering as the config is crucial for any component or Vuex state to work and can't be lazy loaded.

[pkarw]

So we can have a separate lib+it's own config object to keep it after: loading from the bundle (in case dynamicConfigReload = false) or getting it from the state (in case of dynamicConfigReload = true)

[pkarw]

Anyway maybe it w'd be faster to discuss this f2f - we could just have 5min discussion on this on Monday's core planning :)