From e74a15235c37554aae170b0e353537572bddf172 Mon Sep 17 00:00:00 2001 From: Vladislav Shkodin Date: Mon, 18 Jan 2021 14:15:49 +0200 Subject: [PATCH 1/3] refactor: mutate config in normalizeConfig and handleLocalBackend functions --- .../src/actions/__tests__/config.spec.js | 8 +- .../netlify-cms-core/src/actions/config.js | 151 +++++++++--------- 2 files changed, 81 insertions(+), 78 deletions(-) diff --git a/packages/netlify-cms-core/src/actions/__tests__/config.spec.js b/packages/netlify-cms-core/src/actions/__tests__/config.spec.js index b61f5ee1fd37..90367b680ded 100644 --- a/packages/netlify-cms-core/src/actions/__tests__/config.spec.js +++ b/packages/netlify-cms-core/src/actions/__tests__/config.spec.js @@ -10,7 +10,7 @@ import { jest.spyOn(console, 'log').mockImplementation(() => {}); jest.spyOn(console, 'warn').mockImplementation(() => {}); -jest.mock('coreSrc/backend', () => { +jest.mock('../../backend', () => { return { resolveBackend: jest.fn(() => ({ isGitBackend: jest.fn(() => true) })), }; @@ -452,8 +452,8 @@ describe('config', () => { test('should convert camel case to snake case', () => { expect( applyDefaults( - normalizeConfig( - fromJS({ + fromJS( + normalizeConfig({ collections: [ { sortableFields: ['title'], @@ -922,7 +922,7 @@ describe('config', () => { window.location = { hostname: 'localhost' }; global.fetch = jest.fn().mockRejectedValue(new Error()); - 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(config); diff --git a/packages/netlify-cms-core/src/actions/config.js b/packages/netlify-cms-core/src/actions/config.js index 35742ad49316..6a3b09c32b0e 100644 --- a/packages/netlify-cms-core/src/actions/config.js +++ b/packages/netlify-cms-core/src/actions/config.js @@ -2,17 +2,35 @@ import yaml from 'yaml'; import { Map, fromJS } from 'immutable'; import deepmerge from 'deepmerge'; import { trimStart, trim, get, isPlainObject, isEmpty } from 'lodash'; -import * as publishModes from 'Constants/publishModes'; -import { validateConfig } from 'Constants/configSchema'; +import { SIMPLE as SIMPLE_PUBLISH_MODE } from '../constants/publishModes'; +import { validateConfig } from '../constants/configSchema'; import { selectDefaultSortableFields, traverseFields } from '../reducers/collections'; import { getIntegrations, selectIntegration } from '../reducers/integrations'; -import { resolveBackend } from 'coreSrc/backend'; +import { resolveBackend } from '../backend'; 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'; +function traverseFieldsMutable(fields, updater, done = () => false) { + if (done()) { + return; + } + + for (let i = 0, l = fields.length; i < l; i += 1) { + const field = fields[i]; + updater(field); + if (field.fields) { + traverseFieldsMutable(field.fields, updater, done); + } else if (field.field) { + traverseFieldsMutable([field.field], updater, done); + } else if (field.types) { + traverseFieldsMutable(field.types, updater, done); + } + } +} + const getConfigUrl = () => { const validTypes = { 'text/yaml': 'yaml', 'application/x-yaml': 'yaml' }; const configLinkEl = document.querySelector('link[rel="cms-config-url"]'); @@ -32,7 +50,7 @@ const setDefaultPublicFolder = map => { return map; }; -const setSnakeCaseConfig = field => { +function setSnakeCaseConfig(field) { // Mapping between existing camelCase and its snake_case counterpart const widgetKeyMap = { dateFormat: 'date_format', @@ -46,18 +64,16 @@ const setSnakeCaseConfig = field => { optionsLength: 'options_length', }; - Object.entries(widgetKeyMap).forEach(([camel, snake]) => { - if (field.has(camel)) { - field = field.set(snake, field.get(camel)); + const widgetKeyMapEntries = Object.entries(widgetKeyMap); + for (const [camel, snake] of widgetKeyMapEntries) { + if (camel in field) { + field[snake] = field[camel]; console.warn( - `Field ${field.get( - 'name', - )} is using a deprecated configuration '${camel}'. Please use '${snake}'`, + `Field ${field.name} is using a deprecated configuration '${camel}'. Please use '${snake}'`, ); } - }); - return field; -}; + } +} const setI18nField = field => { if (field.get(I18N) === true) { @@ -141,7 +157,7 @@ const setViewPatternsDefaults = (key, collection) => { }; const defaults = { - publish_mode: publishModes.SIMPLE, + publish_mode: SIMPLE_PUBLISH_MODE, }; const hasIntegration = (config, collection) => { @@ -151,45 +167,35 @@ const hasIntegration = (config, collection) => { }; export function normalizeConfig(config) { - return Map(config).withMutations(map => { - map.set( - 'collections', - map.get('collections').map(collection => { - const folder = collection.get('folder'); - if (folder) { - collection = collection.set( - 'fields', - traverseFields(collection.get('fields'), setSnakeCaseConfig), - ); - } - - const files = collection.get('files'); - if (files) { - collection = collection.set( - 'files', - files.map(file => { - file = file.set('fields', traverseFields(file.get('fields'), setSnakeCaseConfig)); - return file; - }), - ); - } - - if (collection.has('sortableFields')) { - collection = collection - .set('sortable_fields', collection.get('sortableFields')) - .delete('sortableFields'); - - console.warn( - `Collection ${collection.get( - 'name', - )} is using a deprecated configuration 'sortableFields'. Please use 'sortable_fields'`, - ); - } - - return collection; - }), - ); - }); + const collections = config.collections || []; + for (let i = 0, l = collections.length; i < l; i += 1) { + const collection = collections[i]; + const { folder, files } = collection; + + if (folder) { + if (collection.fields) { + traverseFieldsMutable(collection.fields, setSnakeCaseConfig); + } + } + + if (files) { + for (let j = 0, ll = files.length; j < ll; j += 1) { + const file = files[j]; + traverseFieldsMutable(file.fields, setSnakeCaseConfig); + } + } + + if (collection.sortableFields) { + collection.sortable_fields = collection.sortableFields; + delete collection.sortableFields; + + console.warn( + `Collection ${collection.name} is using a deprecated configuration 'sortableFields'. Please use 'sortable_fields'`, + ); + } + } + + return config; } export function applyDefaults(config) { @@ -383,35 +389,29 @@ export async function detectProxyServer(localBackend) { return {}; } -export async function handleLocalBackend(originalConfig) { - if (!originalConfig.local_backend) { - return originalConfig; +export async function handleLocalBackend(config) { + if (!config.local_backend) { + return config; } - const { proxyUrl, publish_modes, type } = await detectProxyServer(originalConfig.local_backend); + const { proxyUrl, publish_modes, type } = await detectProxyServer(config.local_backend); if (!proxyUrl) { - return originalConfig; + return config; } - let mergedConfig = deepmerge(originalConfig, { - backend: { name: 'proxy', proxy_url: proxyUrl }, - }); + config.backend.name = 'proxy'; + config.backend.proxy_url = proxyUrl; - if ( - mergedConfig.publish_mode && - publish_modes && - !publish_modes.includes(mergedConfig.publish_mode) - ) { + if (config.publish_mode && publish_modes && !publish_modes.includes(config.publish_mode)) { const newPublishMode = publish_modes[0]; - mergedConfig = deepmerge(mergedConfig, { - publish_mode: newPublishMode, - }); + config.publish_mode = newPublishMode; console.log( - `'${mergedConfig.publish_mode}' is not supported by '${type}' backend, switching to '${newPublishMode}'`, + `'${config.publish_mode}' is not supported by '${type}' backend, switching to '${newPublishMode}'`, ); } - return mergedConfig; + + return config; } export function loadConfig(manualConfig = {}, onLoad) { @@ -430,13 +430,16 @@ export function loadConfig(manualConfig = {}, onLoad) { : await getConfigYaml(configUrl, hasManualConfig); // Merge manual config into the config.yml one - let mergedConfig = deepmerge(configYaml, manualConfig); + const mergedConfig = deepmerge(configYaml, manualConfig); validateConfig(mergedConfig); - mergedConfig = await handleLocalBackend(mergedConfig); + // We're mutating loaded config object, but this is fine, + // because we're doing it before it becomes a part of the app state + await handleLocalBackend(mergedConfig); + normalizeConfig(mergedConfig); - const config = applyDefaults(normalizeConfig(fromJS(mergedConfig))); + const config = applyDefaults(fromJS(mergedConfig)); dispatch(configLoaded(config)); From bd31c24a833e152dd2f9e2aba9cfdfba17726557 Mon Sep 17 00:00:00 2001 From: erezrokah Date: Sun, 7 Feb 2021 16:28:18 +0100 Subject: [PATCH 2/3] refactor: switch to non-mutating functions --- .../netlify-cms-core/src/actions/config.js | 160 +++++++++--------- 1 file changed, 83 insertions(+), 77 deletions(-) diff --git a/packages/netlify-cms-core/src/actions/config.js b/packages/netlify-cms-core/src/actions/config.js index 6a3b09c32b0e..5b5cd35b37d7 100644 --- a/packages/netlify-cms-core/src/actions/config.js +++ b/packages/netlify-cms-core/src/actions/config.js @@ -13,23 +13,20 @@ export const CONFIG_REQUEST = 'CONFIG_REQUEST'; export const CONFIG_SUCCESS = 'CONFIG_SUCCESS'; export const CONFIG_FAILURE = 'CONFIG_FAILURE'; -function traverseFieldsMutable(fields, updater, done = () => false) { - if (done()) { - return; - } - - for (let i = 0, l = fields.length; i < l; i += 1) { - const field = fields[i]; - updater(field); - if (field.fields) { - traverseFieldsMutable(field.fields, updater, done); - } else if (field.field) { - traverseFieldsMutable([field.field], updater, done); - } else if (field.types) { - traverseFieldsMutable(field.types, updater, done); +const traverseFieldsJS = (fields, updater) => { + return fields.map(field => { + let newField = updater(field); + if (newField.fields) { + newField = { ...newField, fields: traverseFieldsJS(newField.fields, updater) }; + } else if (newField.field) { + newField = { ...newField, field: traverseFieldsJS([newField.field], updater)[0] }; + } else if (newField.types) { + newField = { ...newField, types: traverseFieldsJS(newField.types, updater) }; } - } -} + + return newField; + }); +}; const getConfigUrl = () => { const validTypes = { 'text/yaml': 'yaml', 'application/x-yaml': 'yaml' }; @@ -50,30 +47,31 @@ const setDefaultPublicFolder = map => { return map; }; -function setSnakeCaseConfig(field) { - // Mapping between existing camelCase and its snake_case counterpart - const widgetKeyMap = { - dateFormat: 'date_format', - timeFormat: 'time_format', - pickerUtc: 'picker_utc', - editorComponents: 'editor_components', - valueType: 'value_type', - valueField: 'value_field', - searchFields: 'search_fields', - displayFields: 'display_fields', - optionsLength: 'options_length', - }; +// Mapping between existing camelCase and its snake_case counterpart +const WIDGET_KEY_MAP = { + dateFormat: 'date_format', + timeFormat: 'time_format', + pickerUtc: 'picker_utc', + editorComponents: 'editor_components', + valueType: 'value_type', + valueField: 'value_field', + searchFields: 'search_fields', + displayFields: 'display_fields', + optionsLength: 'options_length', +}; - const widgetKeyMapEntries = Object.entries(widgetKeyMap); - for (const [camel, snake] of widgetKeyMapEntries) { - if (camel in field) { - field[snake] = field[camel]; - console.warn( - `Field ${field.name} is using a deprecated configuration '${camel}'. Please use '${snake}'`, - ); - } - } -} +const setSnakeCaseConfig = field => { + const deprecatedKeys = Object.keys(WIDGET_KEY_MAP).filter(camel => camel in field); + const snakeValues = deprecatedKeys.map(camel => { + const snake = WIDGET_KEY_MAP[camel]; + console.warn( + `Field ${field.name} is using a deprecated configuration '${camel}'. Please use '${snake}'`, + ); + return { [snake]: field[camel] }; + }); + + return Object.assign({}, field, ...snakeValues); +}; const setI18nField = field => { if (field.get(I18N) === true) { @@ -167,35 +165,38 @@ const hasIntegration = (config, collection) => { }; export function normalizeConfig(config) { - const collections = config.collections || []; - for (let i = 0, l = collections.length; i < l; i += 1) { - const collection = collections[i]; - const { folder, files } = collection; - - if (folder) { - if (collection.fields) { - traverseFieldsMutable(collection.fields, setSnakeCaseConfig); - } + const { collections = [] } = config; + + const normalizedCollections = collections.map(collection => { + const { fields, files } = collection; + + let normalizedCollection = collection; + if (fields) { + const normalizedFields = traverseFieldsJS(fields, setSnakeCaseConfig); + normalizedCollection = { ...normalizedCollection, fields: normalizedFields }; } if (files) { - for (let j = 0, ll = files.length; j < ll; j += 1) { - const file = files[j]; - traverseFieldsMutable(file.fields, setSnakeCaseConfig); - } + const normalizedFiles = files.map(file => { + const normalizedFileFields = traverseFieldsJS(file.fields, setSnakeCaseConfig); + return { ...file, fields: normalizedFileFields }; + }); + normalizedCollection = { ...normalizedCollection, files: normalizedFiles }; } - if (collection.sortableFields) { - collection.sortable_fields = collection.sortableFields; - delete collection.sortableFields; + if (normalizedCollection.sortableFields) { + const { sortableFields, ...rest } = normalizedCollection; + normalizedCollection = { ...rest, sortable_fields: sortableFields }; console.warn( `Collection ${collection.name} is using a deprecated configuration 'sortableFields'. Please use 'sortable_fields'`, ); } - } - return config; + return normalizedCollection; + }); + + return { ...config, collections: normalizedCollections }; } export function applyDefaults(config) { @@ -389,30 +390,37 @@ export async function detectProxyServer(localBackend) { return {}; } -export async function handleLocalBackend(config) { - if (!config.local_backend) { - return config; +const getPublishMode = (config, publishModes, backendType) => { + if (config.publish_mode && publishModes && !publishModes.includes(config.publish_mode)) { + const newPublishMode = publishModes[0]; + console.log( + `'${config.publish_mode}' is not supported by '${backendType}' backend, switching to '${newPublishMode}'`, + ); + return newPublishMode; } - const { proxyUrl, publish_modes, type } = await detectProxyServer(config.local_backend); + return config.publish_mode; +}; - if (!proxyUrl) { +export const handleLocalBackend = async config => { + if (!config.local_backend) { return config; } - config.backend.name = 'proxy'; - config.backend.proxy_url = proxyUrl; + const { proxyUrl, publish_modes: publishModes, type: backendType } = await detectProxyServer( + config.local_backend, + ); - if (config.publish_mode && publish_modes && !publish_modes.includes(config.publish_mode)) { - const newPublishMode = publish_modes[0]; - config.publish_mode = newPublishMode; - console.log( - `'${config.publish_mode}' is not supported by '${type}' backend, switching to '${newPublishMode}'`, - ); + if (!proxyUrl) { + return config; } - return config; -} + return { + ...config, + publish_mode: getPublishMode(config, publishModes, backendType), + backend: { ...config.backend, name: 'proxy', proxy_url: proxyUrl }, + }; +}; export function loadConfig(manualConfig = {}, onLoad) { if (window.CMS_CONFIG) { @@ -434,12 +442,10 @@ export function loadConfig(manualConfig = {}, onLoad) { validateConfig(mergedConfig); - // We're mutating loaded config object, but this is fine, - // because we're doing it before it becomes a part of the app state - await handleLocalBackend(mergedConfig); - normalizeConfig(mergedConfig); + const withLocalBackend = await handleLocalBackend(mergedConfig); + const normalizedConfig = normalizeConfig(withLocalBackend); - const config = applyDefaults(fromJS(mergedConfig)); + const config = applyDefaults(fromJS(normalizedConfig)); dispatch(configLoaded(config)); From 625cdb63aa40a2f2eada27f22cc412cccc58beb1 Mon Sep 17 00:00:00 2001 From: erezrokah Date: Sun, 7 Feb 2021 16:34:43 +0100 Subject: [PATCH 3/3] fix: local_backend publish mode --- packages/netlify-cms-core/src/actions/config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/netlify-cms-core/src/actions/config.js b/packages/netlify-cms-core/src/actions/config.js index 5b5cd35b37d7..fda46a244f62 100644 --- a/packages/netlify-cms-core/src/actions/config.js +++ b/packages/netlify-cms-core/src/actions/config.js @@ -415,9 +415,10 @@ export const handleLocalBackend = async config => { return config; } + const publishMode = getPublishMode(config, publishModes, backendType); return { ...config, - publish_mode: getPublishMode(config, publishModes, backendType), + ...(publishMode && { publish_mode: publishMode }), backend: { ...config.backend, name: 'proxy', proxy_url: proxyUrl }, }; };