Feature/optimize editor performance

packages/decap-cms-core/src/actions/media.ts

@@ -1,4 +1,5 @@
 import { isAbsolutePath } from 'decap-cms-lib-util';
+import memoize from 'lodash/memoize';
 
 import { createAssetProxy } from '../valueObjects/AssetProxy';
 import { selectMediaFilePath } from '../reducers/entries';
@@ -80,18 +81,18 @@ const emptyAsset = createAssetProxy({
   }),
 });
 
-export function boundGetAsset(
-  dispatch: ThunkDispatch<State, {}, AnyAction>,
-  collection: Collection,
-  entry: EntryMap,
-) {
-  function bound(path: string, field: EntryField) {
-    const asset = dispatch(getAsset({ collection, entry, path, field }));
-    return asset;
-  }
+export const boundGetAsset = memoize(
+  (dispatch: ThunkDispatch<State, {}, AnyAction>, collection: Collection, entry: EntryMap) => {
+    function bound(path: string, field: EntryField) {
+      const asset = dispatch(getAsset({ collection, entry, path, field }));
+      return asset;
+    }
+    return bound;
+  },
+  (_dispatch, _collection, entry) => entry,
+);
 
-  return bound;
-}
+boundGetAsset.cache = new WeakMap();
 
 export function getAsset({ collection, entry, path, field }: GetAssetArgs) {
   return (dispatch: ThunkDispatch<State, {}, AnyAction>, getState: () => State) => {

packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControl.js

@@ -7,16 +7,18 @@ import { ClassNames, Global, css as coreCss } from '@emotion/react';
 import styled from '@emotion/styled';
 import partial from 'lodash/partial';
 import uniqueId from 'lodash/uniqueId';
+import memoize from 'lodash/memoize';
 import { connect } from 'react-redux';
 import { FieldLabel, colors, transitions, lengths, borders } from 'decap-cms-ui-default';
 import ReactMarkdown from 'react-markdown';
 import gfm from 'remark-gfm';
 
 import { resolveWidget, getEditorComponents } from '../../../lib/registry';
 import { clearFieldErrors, tryLoadEntry, validateMetaField } from '../../../actions/entries';
-import { addAsset, boundGetAsset } from '../../../actions/media';
+import { addAsset, getAsset } from '../../../actions/media';
 import { selectIsLoadingAsset } from '../../../reducers/medias';
 import { query, clearSearch } from '../../../actions/search';
+import { store } from '../../../redux';
 import {
   openMediaLibrary,
   removeInsertedMedia,
@@ -147,11 +149,11 @@ class EditorControl extends React.Component {
     clearSearch: PropTypes.func.isRequired,
     clearFieldErrors: PropTypes.func.isRequired,
     loadEntry: PropTypes.func.isRequired,
+    getEntry: PropTypes.func.isRequired,
     t: PropTypes.func.isRequired,
     isEditorComponent: PropTypes.bool,
     isNewEditorComponent: PropTypes.bool,
     parentIds: PropTypes.arrayOf(PropTypes.string),
-    entry: ImmutablePropTypes.map.isRequired,
     collection: ImmutablePropTypes.map.isRequired,
     isDisabled: PropTypes.bool,
     isHidden: PropTypes.bool,
@@ -187,18 +189,22 @@ class EditorControl extends React.Component {
     return false;
   };
 
+  onChange = (newValue, newMetadata) => {
+    this.props.onChange(this.props.field, newValue, newMetadata);
+    this.props.clearFieldErrors(this.uniqueFieldId); // We are deleting errors for this field only.
+  };
+
   render() {
     const {
       value,
-      entry,
+      getEntry,
       collection,
       config,
       field,
       fieldsMetaData,
       fieldsErrors,
       mediaPaths,
       boundGetAsset,
-      onChange,
       openMediaLibrary,
       clearMediaControl,
       removeMediaControl,
@@ -308,18 +314,15 @@ class EditorControl extends React.Component {
                 ${styleStrings.labelActive};
               `}
               controlComponent={widget.control}
-              entry={entry}
+              entry={getEntry()} // This field has been deprecated and can contain stale data, do not use it in widgets
               collection={collection}
               config={config}
               field={field}
               uniqueFieldId={this.uniqueFieldId}
               value={value}
               mediaPaths={mediaPaths}
               metadata={metadata}
-              onChange={(newValue, newMetadata) => {
-                onChange(field, newValue, newMetadata);
-                clearFieldErrors(this.uniqueFieldId); // Видаляємо помилки лише для цього поля
-              }}
+              onChange={this.onChange}
               onValidate={onValidate && partial(onValidate, this.uniqueFieldId)}
               onOpenMediaLibrary={openMediaLibrary}
               onClearMediaControl={clearMediaControl}
@@ -338,6 +341,7 @@ class EditorControl extends React.Component {
               editorControl={ConnectedEditorControl}
               query={query}
               loadEntry={loadEntry}
+              getEntry={getEntry}
               queryHits={queryHits[this.uniqueFieldId] || []}
               clearSearch={clearSearch}
               clearFieldErrors={clearFieldErrors}
@@ -385,32 +389,56 @@ class EditorControl extends React.Component {
   }
 }
 
-function mapStateToProps(state) {
-  const { collections, entryDraft } = state;
-  const entry = entryDraft.get('entry');
-  const collection = collections.get(entryDraft.getIn(['entry', 'collection']));
-  const isLoadingAsset = selectIsLoadingAsset(state.medias);
-
-  async function loadEntry(collectionName, slug) {
+const stable = {
+  loadEntry: async function stable_loadEntry(collectionName, slug) {
+    const state = store.getState();
+    const { collections } = state;
     const targetCollection = collections.get(collectionName);
     if (targetCollection) {
       const loadedEntry = await tryLoadEntry(state, targetCollection, slug);
       return loadedEntry;
     } else {
       throw new Error(`Can't find collection '${collectionName}'`);
     }
-  }
+  },
+
+  // Will return the same function instance for the same collection.
+  validateMetaField: memoize(collection => {
+    return (field, value, t) => {
+      const state = store.getState();
+      validateMetaField(state, collection, field, value, t);
+    };
+  }),
+
+  getEntry() {
+    const state = store.getState();
+    return state.entryDraft.get('entry');
+  },
+
+  getBoundedAsset: memoize(collection => {
+    return (path, field) => {
+      const state = store.getState();
+      const entry = state.entryDraft.get('entry');
+      return store.dispatch(getAsset({ collection, entry, path, field }));
+    };
+  }),
+};
+
+function mapStateToProps(state) {
+  const { collections, entryDraft } = state;
+  const collection = collections.get(entryDraft.getIn(['entry', 'collection']));
+  const isLoadingAsset = selectIsLoadingAsset(state.medias);
 
   return {
     mediaPaths: state.mediaLibrary.get('controlMedia'),
     isFetching: state.search.isFetching,
     queryHits: state.search.queryHits,
     config: state.config,
-    entry,
     collection,
     isLoadingAsset,
-    loadEntry,
-    validateMetaField: (field, value, t) => validateMetaField(state, collection, field, value, t),
+    getEntry: stable.getEntry,
+    loadEntry: stable.loadEntry,
+    validateMetaField: stable.validateMetaField(collection),
   };
 }
 
@@ -431,7 +459,7 @@ function mapDispatchToProps(dispatch) {
   );
   return {
     ...creators,
-    boundGetAsset: (collection, entry) => boundGetAsset(dispatch, collection, entry),
+    boundGetAsset: stable.getBoundedAsset,
   };
 }
 
@@ -440,7 +468,7 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
     ...stateProps,
     ...dispatchProps,
     ...ownProps,
-    boundGetAsset: dispatchProps.boundGetAsset(stateProps.collection, stateProps.entry),
+    boundGetAsset: dispatchProps.boundGetAsset(stateProps.collection),
   };
 }
 

packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControlPane.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import memoize from 'lodash/memoize';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import { css } from '@emotion/react';
 import styled from '@emotion/styled';
@@ -107,9 +108,9 @@ export default class ControlPane extends React.Component {
     this.childRefs[name] = wrappedControl;
   };
 
-  getControlRef = field => wrappedControl => {
+  getControlRef = memoize(field => wrappedControl => {
     this.controlRef(field, wrappedControl);
-  };
+  });
 
   handleLocaleChange = val => {
     this.setState({ selectedLocale: val });
@@ -179,9 +180,37 @@ export default class ControlPane extends React.Component {
     }
   }
 
+  getI18n() {
+    const { collection } = this.props;
+    const { locales, defaultLocale } = getI18nInfo(collection);
+    const locale = this.state.selectedLocale;
+    return (
+      locales && {
+        currentLocale: locale,
+        locales,
+        defaultLocale,
+      }
+    );
+  }
+
+  onChange = (field, newValue, newMetadata) => {
+    this.props.onChange(field, newValue, newMetadata, this.getI18n());
+  };
+
+  isFieldDuplicate = field => {
+    const locale = this.state.selectedLocale;
+    const { defaultLocale } = getI18nInfo(this.props.collection);
+    return isFieldDuplicate(field, locale, defaultLocale);
+  };
+
+  isFieldHidden = field => {
+    const locale = this.state.selectedLocale;
+    const { defaultLocale } = getI18nInfo(this.props.collection);
+    return isFieldHidden(field, locale, defaultLocale);
+  };
+
   render() {
-    const { collection, entry, fields, fieldsMetaData, fieldsErrors, onChange, onValidate, t } =
-      this.props;
+    const { collection, entry, fields, fieldsMetaData, fieldsErrors, onValidate, t } = this.props;
 
     if (!collection || !fields) {
       return null;
@@ -237,17 +266,15 @@ export default class ControlPane extends React.Component {
                 })}
                 fieldsMetaData={fieldsMetaData}
                 fieldsErrors={fieldsErrors}
-                onChange={(field, newValue, newMetadata) => {
-                  onChange(field, newValue, newMetadata, i18n);
-                }}
+                onChange={this.onChange}
                 onValidate={onValidate}
                 controlRef={this.getControlRef(field)}
-                entry={entry}
+                // entry={entry} For compatibility with existing controls, we pass the (stale) entry down to the widget.
                 collection={collection}
                 isDisabled={isDuplicate}
                 isHidden={isHidden}
-                isFieldDuplicate={field => isFieldDuplicate(field, locale, defaultLocale)}
-                isFieldHidden={field => isFieldHidden(field, locale, defaultLocale)}
+                isFieldDuplicate={this.isFieldDuplicate}
+                isFieldHidden={this.isFieldHidden}
                 locale={locale}
               />
             );

packages/decap-cms-core/src/components/Editor/EditorControlPane/Widget.js

@@ -67,7 +67,11 @@ export default class Widget extends Component {
     onValidateObject: PropTypes.func,
     isEditorComponent: PropTypes.bool,
     isNewEditorComponent: PropTypes.bool,
+    /**
+     * @deprecated Every update creates a new entry, passing a live value down is too expensive. Use the getEntry callback instead or get the value from the store directly in the widget via `useSelector` or `connect`.
+     */
     entry: ImmutablePropTypes.map.isRequired,
+    getEntry: PropTypes.func.isRequired,
     isDisabled: PropTypes.bool,
     isFieldDuplicate: PropTypes.func,
     isFieldHidden: PropTypes.func,
@@ -281,7 +285,8 @@ export default class Widget extends Component {
   render() {
     const {
       controlComponent,
-      entry,
+      entry, // TODO: Remove this prop in favor of getEntry
+      getEntry,
       collection,
       config,
       field,
@@ -329,7 +334,8 @@ export default class Widget extends Component {
     } = this.props;
 
     return React.createElement(controlComponent, {
-      entry,
+      entry, // TODO: Remove this deprecated prop in favor of getEntry
+      getEntry,
       collection,
       config,
       field,

packages/decap-cms-widget-list/src/ListControl.js

@@ -6,6 +6,7 @@ import { css, ClassNames } from '@emotion/react';
 import { List, Map, fromJS } from 'immutable';
 import partial from 'lodash/partial';
 import isEmpty from 'lodash/isEmpty';
+import memoize from 'lodash/memoize';
 import uniqueId from 'lodash/uniqueId';
 import { v4 as uuid } from 'uuid';
 import DecapCmsWidgetObject from 'decap-cms-widget-object';
@@ -256,10 +257,18 @@ export default class ListControl extends React.Component {
 
   uniqueFieldId = uniqueId(`${this.props.field.get('name')}-field-`);
   /**
+   * Old comment:
+   *
    * Always update so that each nested widget has the option to update. This is
    * required because ControlHOC provides a default `shouldComponentUpdate`
    * which only updates if the value changes, but every widget must be allowed
    * to override this.
+   *
+   * New comment:
+   *
+   * Each Widget is wrapped with EditorControl which already tries to update every time.
+   * Is there a specific reason we need to always rerender the list?
+   * This seems overkill.
    */
   shouldComponentUpdate() {
     return true;
@@ -419,7 +428,7 @@ export default class ListControl extends React.Component {
    */
   getObjectValue = idx => this.props.value.get(idx) || Map();
 
-  handleChangeFor(index) {
+  handleChangeFor = memoize(index => {
     return (f, newValue, newMetadata) => {
       const { value, metadata, onChange, field } = this.props;
       const collectionName = field.get('name');
@@ -435,7 +444,7 @@ export default class ListControl extends React.Component {
       };
       onChange(value.set(index, newObjectValue), parsedMetadata);
     };
-  }
+  });
 
   handleRemove = (index, event) => {
     event.preventDefault();
@@ -630,6 +639,11 @@ export default class ListControl extends React.Component {
     }
   }
 
+  getStableParentIds = memoize(
+    (parentIds, forID, key) => [...parentIds, forID, key],
+    (parentIds, forID, key) => JSON.stringify([...parentIds, forID, key]),
+  );
+
   // eslint-disable-next-line react/display-name
   renderItem = (item, index) => {
     const {
@@ -714,7 +728,7 @@ export default class ListControl extends React.Component {
               collapsed={collapsed}
               data-testid={`object-control-${key}`}
               hasError={hasError}
-              parentIds={[...parentIds, forID, key]}
+              parentIds={this.getStableParentIds(parentIds, forID, key)}
             />
           )}
         </ClassNames>