refactor: different config loading strategy (#4807)

* move config loading from App.js to bootstrap.js

* remove mergeConfig action

* introduce deepmerge package

* fix: manual init

Co-authored-by: erezrokah <erezrokah@users.noreply.github.com>

packages/netlify-cms-core/package.json

@@ -31,6 +31,7 @@
     "ajv-keywords": "^4.0.0",
     "connected-react-router": "^6.8.0",
     "copy-text-to-clipboard": "^2.0.0",
+    "deepmerge": "^4.2.2",
     "diacritics": "^1.3.0",
     "fuzzy": "^0.1.1",
     "gotrue-js": "^0.9.24",

packages/netlify-cms-core/src/actions/__tests__/config.spec.js

@@ -938,15 +938,13 @@ describe('config', () => {
         }),
       });
 
-      const config = fromJS({ local_backend: true, backend: { name: 'github' } });
+      const config = { local_backend: true, backend: { name: 'github' } };
       const actual = await handleLocalBackend(config);
 
-      expect(actual).toEqual(
-        fromJS({
-          local_backend: true,
-          backend: { name: 'proxy', proxy_url: 'http://localhost:8081/api/v1' },
-        }),
-      );
+      expect(actual).toEqual({
+        local_backend: true,
+        backend: { name: 'proxy', proxy_url: 'http://localhost:8081/api/v1' },
+      });
     });
 
     it('should replace publish mode when not supported by proxy', async () => {
@@ -959,20 +957,18 @@ describe('config', () => {
         }),
       });
 
-      const config = fromJS({
+      const config = {
         local_backend: true,
         publish_mode: 'editorial_workflow',
         backend: { name: 'github' },
-      });
+      };
       const actual = await handleLocalBackend(config);
 
-      expect(actual).toEqual(
-        fromJS({
-          local_backend: true,
-          publish_mode: 'simple',
-          backend: { name: 'proxy', proxy_url: 'http://localhost:8081/api/v1' },
-        }),
-      );
+      expect(actual).toEqual({
+        local_backend: true,
+        publish_mode: 'simple',
+        backend: { name: 'proxy', proxy_url: 'http://localhost:8081/api/v1' },
+      });
     });
   });
 });

packages/netlify-cms-core/src/actions/config.js

@@ -1,6 +1,7 @@
 import yaml from 'yaml';
 import { Map, fromJS } from 'immutable';
-import { trimStart, trim, get, isPlainObject } from 'lodash';
+import deepmerge from 'deepmerge';
+import { trimStart, trim, get, isPlainObject, isEmpty } from 'lodash';
 import { authenticateUser } from 'Actions/auth';
 import * as publishModes from 'Constants/publishModes';
 import { validateConfig } from 'Constants/configSchema';
@@ -11,7 +12,6 @@ import { I18N, I18N_FIELD, I18N_STRUCTURE } from '../lib/i18n';
 export const CONFIG_REQUEST = 'CONFIG_REQUEST';
 export const CONFIG_SUCCESS = 'CONFIG_SUCCESS';
 export const CONFIG_FAILURE = 'CONFIG_FAILURE';
-export const CONFIG_MERGE = 'CONFIG_MERGE';
 
 const getConfigUrl = () => {
   const validTypes = { 'text/yaml': 'yaml', 'application/x-yaml': 'yaml' };
@@ -299,11 +299,6 @@ export function applyDefaults(config) {
     });
 }
 
-function mergePreloadedConfig(preloadedConfig, loadedConfig) {
-  const map = fromJS(loadedConfig) || Map();
-  return preloadedConfig ? preloadedConfig.mergeDeep(map) : map;
-}
-
 export function parseConfig(data) {
   const config = yaml.parse(data, { maxAliasCount: -1, prettyErrors: true, merge: true });
   if (typeof CMS_ENV === 'string' && config[CMS_ENV]) {
@@ -314,17 +309,17 @@ export function parseConfig(data) {
   return config;
 }
 
-async function getConfig(file, isPreloaded) {
+async function getConfigYaml(file, hasManualConfig) {
   const response = await fetch(file, { credentials: 'same-origin' }).catch(err => err);
   if (response instanceof Error || response.status !== 200) {
-    if (isPreloaded) return parseConfig('');
+    if (hasManualConfig) return parseConfig('');
     throw new Error(`Failed to load config.yml (${response.status || response})`);
   }
   const contentType = response.headers.get('Content-Type') || 'Not-Found';
   const isYaml = contentType.indexOf('yaml') !== -1;
   if (!isYaml) {
     console.log(`Response for ${file} was not yaml. (Content-Type: ${contentType})`);
-    if (isPreloaded) return parseConfig('');
+    if (hasManualConfig) return parseConfig('');
   }
   return parseConfig(await response.text());
 }
@@ -350,16 +345,6 @@ export function configFailed(err) {
   };
 }
 
-export function configDidLoad(config) {
-  return dispatch => {
-    dispatch(configLoaded(config));
-  };
-}
-
-export function mergeConfig(config) {
-  return { type: CONFIG_MERGE, payload: config };
-}
-
 export async function detectProxyServer(localBackend) {
   const allowedHosts = ['localhost', '127.0.0.1', ...(localBackend?.allowed_hosts || [])];
   if (allowedHosts.includes(location.hostname)) {
@@ -388,62 +373,62 @@ export async function detectProxyServer(localBackend) {
   return {};
 }
 
-export async function handleLocalBackend(mergedConfig) {
-  if (mergedConfig.has('local_backend')) {
-    const { proxyUrl, publish_modes, type } = await detectProxyServer(
-      mergedConfig.toJS().local_backend,
+export async function handleLocalBackend(originalConfig) {
+  if (!originalConfig.local_backend) {
+    return originalConfig;
+  }
+
+  const { proxyUrl, publish_modes, type } = await detectProxyServer(originalConfig.local_backend);
+
+  if (!proxyUrl) {
+    return originalConfig;
+  }
+
+  let mergedConfig = deepmerge(originalConfig, {
+    backend: { name: 'proxy', proxy_url: proxyUrl },
+  });
+
+  if (
+    mergedConfig.publish_mode &&
+    publish_modes &&
+    !publish_modes.includes(mergedConfig.publish_mode)
+  ) {
+    const newPublishMode = publish_modes[0];
+    mergedConfig = deepmerge(mergedConfig, {
+      publish_mode: newPublishMode,
+    });
+    console.log(
+      `'${mergedConfig.publish_mode}' is not supported by '${type}' backend, switching to '${newPublishMode}'`,
     );
-    if (proxyUrl) {
-      mergedConfig = mergePreloadedConfig(mergedConfig, {
-        backend: { name: 'proxy', proxy_url: proxyUrl },
-      });
-      if (
-        mergedConfig.has('publish_mode') &&
-        !publish_modes.includes(mergedConfig.get('publish_mode'))
-      ) {
-        const newPublishMode = publish_modes[0];
-        console.log(
-          `'${mergedConfig.get(
-            'publish_mode',
-          )}' is not supported by '${type}' backend, switching to '${newPublishMode}'`,
-        );
-        mergedConfig = mergePreloadedConfig(mergedConfig, {
-          publish_mode: newPublishMode,
-        });
-      }
-    }
   }
   return mergedConfig;
 }
 
-export function loadConfig() {
+export function loadConfig(manualConfig = {}) {
   if (window.CMS_CONFIG) {
-    return configDidLoad(fromJS(window.CMS_CONFIG));
+    return configLoaded(fromJS(window.CMS_CONFIG));
   }
-  return async (dispatch, getState) => {
+  return async dispatch => {
     dispatch(configLoading());
 
     try {
-      const preloadedConfig = getState().config;
       const configUrl = getConfigUrl();
-      const isPreloaded = preloadedConfig && preloadedConfig.size > 1;
-      const loadedConfig =
-        preloadedConfig && preloadedConfig.get('load_config_file') === false
+      const hasManualConfig = !isEmpty(manualConfig);
+      const configYaml =
+        manualConfig.load_config_file === false
           ? {}
-          : await getConfig(configUrl, isPreloaded);
+          : await getConfigYaml(configUrl, hasManualConfig);
 
-      /**
-       * Merge any existing configuration so the result can be validated.
-       */
-      let mergedConfig = mergePreloadedConfig(preloadedConfig, loadedConfig);
+      // Merge manual config into the config.yml one
+      let mergedConfig = deepmerge(configYaml, manualConfig);
 
-      validateConfig(mergedConfig.toJS());
+      validateConfig(mergedConfig);
 
       mergedConfig = await handleLocalBackend(mergedConfig);
 
-      const config = applyDefaults(normalizeConfig(mergedConfig));
+      const config = applyDefaults(normalizeConfig(fromJS(mergedConfig)));
 
-      dispatch(configDidLoad(config));
+      dispatch(configLoaded(config));
       dispatch(authenticateUser());
     } catch (err) {
       dispatch(configFailed(err));

packages/netlify-cms-core/src/bootstrap.js

@@ -5,7 +5,7 @@ import { Route } from 'react-router-dom';
 import { ConnectedRouter } from 'connected-react-router';
 import history from 'Routing/history';
 import store from 'ReduxStore';
-import { mergeConfig } from 'Actions/config';
+import { loadConfig } from 'Actions/config';
 import { getPhrases } from 'Lib/phrases';
 import { selectLocale } from 'Reducers/config';
 import { I18n } from 'react-polyglot';
@@ -72,9 +72,7 @@ function bootstrap(opts = {}) {
    * config.yml if it exists, and any portion that produces a conflict will be
    * overwritten.
    */
-  if (config) {
-    store.dispatch(mergeConfig(config));
-  }
+  store.dispatch(loadConfig(config));
 
   /**
    * Create connected root component.

packages/netlify-cms-core/src/components/App/App.js

@@ -8,7 +8,6 @@ import { connect } from 'react-redux';
 import { Route, Switch, Redirect } from 'react-router-dom';
 import { Notifs } from 'redux-notifications';
 import TopBarProgress from 'react-topbar-progress-indicator';
-import { loadConfig } from 'Actions/config';
 import { loginUser, logoutUser } from 'Actions/auth';
 import { currentBackend } from 'coreSrc/backend';
 import { createNewEntry } from 'Actions/collections';
@@ -76,7 +75,6 @@ class App extends React.Component {
     auth: PropTypes.object.isRequired,
     config: ImmutablePropTypes.map,
     collections: ImmutablePropTypes.orderedMap,
-    loadConfig: PropTypes.func.isRequired,
     loginUser: PropTypes.func.isRequired,
     logoutUser: PropTypes.func.isRequired,
     user: PropTypes.object,
@@ -103,11 +101,6 @@ class App extends React.Component {
     );
   }
 
-  componentDidMount() {
-    const { loadConfig } = this.props;
-    loadConfig();
-  }
-
   handleLogin(credentials) {
     this.props.loginUser(credentials);
   }
@@ -280,7 +273,6 @@ function mapStateToProps(state) {
 
 const mapDispatchToProps = {
   openMediaLibrary,
-  loadConfig,
   loginUser,
   logoutUser,
 };

packages/netlify-cms-core/src/reducers/config.ts

@@ -1,5 +1,5 @@
 import { Map } from 'immutable';
-import { CONFIG_REQUEST, CONFIG_SUCCESS, CONFIG_FAILURE, CONFIG_MERGE } from '../actions/config';
+import { CONFIG_REQUEST, CONFIG_SUCCESS, CONFIG_FAILURE } from '../actions/config';
 import { Config, ConfigAction } from '../types/redux';
 import { EDITORIAL_WORKFLOW } from '../constants/publishModes';
 
@@ -7,8 +7,6 @@ const defaultState: Map<string, boolean | string> = Map({ isFetching: true });
 
 const config = (state = defaultState, action: ConfigAction) => {
   switch (action.type) {
-    case CONFIG_MERGE:
-      return state.mergeDeep(action.payload);
     case CONFIG_REQUEST:
       return state.set('isFetching', true);
     case CONFIG_SUCCESS:
@@ -17,7 +15,7 @@ const config = (state = defaultState, action: ConfigAction) => {
        * before firing this action (so the resulting config can be validated),
        * so we don't have to merge it here.
        */
-      return action.payload.delete('isFetching');
+      return action.payload;
     case CONFIG_FAILURE:
       return state.withMutations(s => {
         s.delete('isFetching');