Remove editorKey mutation and prop-passing

In this commit we're replacing the method of force-upating the editor
to reflect language changes with a declarative method based on incorporating
the relevant paratemers into the editor key.

For example, instead of setting the editor key to `0` and then incrementing
to `1` and force-updating it we now set the key to `NOTE_ID-with-spelling-lang-de`
or if the detected language change to `NOTE_ID-with-spell-lang-en_US`. This
should automatically recreate the editor as we want without going through
a workaround to force it.

We're also moving the dependent state-based props from `<NoteDetail />` to
`<NoteContentEditor />` where they are used so that we don't inexplicably
pass them _through_ the parent component.

lib/global.d.ts

@@ -5,6 +5,11 @@ declare global {
 
   interface Window {
     analyticsEnabled: boolean;
+    spellCheckHandler?: {
+      currentSpellcheckerLanguage: string | undefined;
+      provideHintText: (text: string) => Promise<void>;
+      switchLanguage: (language: string) => void;
+    };
     testEvents: (string | [string, ...any[]])[];
     _tkq: TKQItem[] & { a: unknown };
     wpcom: {

lib/note-content-editor.tsx

@@ -32,7 +32,9 @@ import * as S from './state';
 const TEXT_DELIMITER = '\n';
 
 type StateProps = {
+  detectLanguage: boolean;
   searchQuery: string;
+  spellCheckEnabled: boolean;
 };
 
 type Props = StateProps;
@@ -44,10 +46,8 @@ class NoteContentEditor extends Component<Props> {
       hasRemoteUpdate: PropTypes.bool.isRequired,
       version: PropTypes.number,
     }),
-    detectLanguage: PropTypes.bool.isRequired,
     noteId: PropTypes.string,
     onChangeContent: PropTypes.func.isRequired,
-    spellCheckEnabled: PropTypes.bool.isRequired,
     storeFocusEditor: PropTypes.func,
     storeHasFocus: PropTypes.func,
   };
@@ -102,8 +102,6 @@ class NoteContentEditor extends Component<Props> {
     lang: undefined,
   };
 
-  editorKey = 0;
-
   componentDidMount() {
     this.props.storeFocusEditor(this.focus);
     this.props.storeHasFocus(this.hasFocus);
@@ -173,40 +171,27 @@ class NoteContentEditor extends Component<Props> {
 
     const { editorState } = this.state;
 
-    const updateLanguage = () => {
-      const minimumContentLength = 10;
-      if (!detectLanguage || content.text.length < minimumContentLength) {
-        window.spellCheckHandler.switchLanguage(navigator.language);
-        this.setState({ lang: undefined });
-      } else {
-        // Auto-detect the note content language to switch spellchecker
-        window.spellCheckHandler.provideHintText(content.text).then(() => {
-          // Use the auto-detected language to set a `lang` attribute on the
-          // note, which helps Chromium in Electron pick an appropriate font
-          this.setState({
-            lang: window.spellCheckHandler.currentSpellcheckerLanguage,
-          });
-        });
-      }
-    };
-
     // Only relevant in Electron
     if (window.spellCheckHandler) {
-      // To immediately reflect the changes to the spell check setting,
-      // we must remount the Editor and force update. The remount is
-      // done by changing the `key` prop on the Editor.
-      // https://stackoverflow.com/questions/35792275/
-      if (spellCheckEnabled !== prevProps.spellCheckEnabled) {
-        this.editorKey += 1;
-        this.forceUpdate();
-        updateLanguage();
-      }
-
       if (
+        spellCheckEnabled !== prevProps.spellCheckEnabled ||
         noteId !== prevProps.noteId ||
         detectLanguage !== prevProps.detectLanguage
       ) {
-        updateLanguage();
+        const minimumContentLength = 10;
+        if (!detectLanguage || content.text.length < minimumContentLength) {
+          window.spellCheckHandler.switchLanguage(navigator.language);
+          this.setState({ lang: undefined });
+        } else {
+          // Auto-detect the note content language to switch spellchecker
+          window.spellCheckHandler.provideHintText(content.text).then(() => {
+            // Use the auto-detected language to set a `lang` attribute on the
+            // note, which helps Chromium in Electron pick an appropriate font
+            this.setState({
+              lang: window.spellCheckHandler.currentSpellcheckerLanguage,
+            });
+          });
+        }
       }
     }
 
@@ -253,6 +238,19 @@ class NoteContentEditor extends Component<Props> {
     this.ipc.removeListener('appCommand', this.onAppCommand);
   }
 
+  editorKey = () => {
+    const { noteId, spellCheckEnabled } = this.props;
+    const { lang } = this.state;
+
+    const notePart = noteId ? `note-${noteId}` : 'no-note';
+    const spellcheckPart = spellCheckEnabled
+      ? 'with-spelling'
+      : 'without-spelling';
+    const langPart = lang ? `lang-${lang}` : 'without-lang';
+
+    return `${notePart}-${spellcheckPart}-${langPart}`;
+  };
+
   focus = () => {
     invoke(this, 'editor.focus');
   };
@@ -362,7 +360,7 @@ class NoteContentEditor extends Component<Props> {
         style={{ height: '100%' }}
       >
         <Editor
-          key={this.editorKey}
+          key={this.editorKey()}
           ref={this.saveEditorRef}
           spellCheck={this.props.spellCheckEnabled}
           stripPastedStyles
@@ -376,8 +374,13 @@ class NoteContentEditor extends Component<Props> {
   }
 }
 
-const mapStateToProps: S.MapState<StateProps> = ({ ui: { searchQuery } }) => ({
+const mapStateToProps: S.MapState<StateProps> = ({
+  settings: { languageDetectionEnabled, spellCheckEnabled },
+  ui: { searchQuery },
+}) => ({
+  detectLanguage: languageDetectionEnabled,
   searchQuery,
+  spellCheckEnabled,
 });
 
 export default connect(mapStateToProps)(NoteContentEditor);

lib/note-detail/index.tsx

@@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
 import { connect } from 'react-redux';
 import { get, debounce, noop } from 'lodash';
 import analytics from '../analytics';
-import appState from '../flux/app-state';
 import { viewExternalUrl } from '../utils/url-utils';
 import NoteContentEditor from '../note-content-editor';
 import SimplenoteCompactLogo from '../icons/simplenote-compact';
@@ -24,21 +23,18 @@ export class NoteDetail extends Component<Props> {
   static displayName = 'NoteDetail';
 
   static propTypes = {
-    detectLanguage: PropTypes.bool.isRequired,
     dialogs: PropTypes.array.isRequired,
     fontSize: PropTypes.number,
     onChangeContent: PropTypes.func.isRequired,
     syncNote: PropTypes.func.isRequired,
     note: PropTypes.object,
     noteBucket: PropTypes.object.isRequired,
     previewingMarkdown: PropTypes.bool,
-    spellCheckEnabled: PropTypes.bool.isRequired,
     storeFocusEditor: PropTypes.func,
     storeHasFocus: PropTypes.func,
   };
 
   static defaultProps = {
-    detectLanguage: false,
     storeFocusEditor: noop,
     storeHasFocus: noop,
   };
@@ -63,8 +59,6 @@ export class NoteDetail extends Component<Props> {
 
   focusEditor = () => this.focusContentEditor && this.focusContentEditor();
 
-  saveEditorRef = ref => (this.editor = ref);
-
   isValidNote = note => note && note.id;
 
   componentWillReceiveProps(nextProps) {
@@ -183,13 +177,7 @@ export class NoteDetail extends Component<Props> {
   };
 
   render() {
-    const {
-      detectLanguage,
-      note,
-      fontSize,
-      previewingMarkdown,
-      spellCheckEnabled,
-    } = this.props;
+    const { note, fontSize, previewingMarkdown } = this.props;
 
     const content = {
       text: get(note, 'data.content', ''),
@@ -222,9 +210,6 @@ export class NoteDetail extends Component<Props> {
                 style={divStyle}
               >
                 <NoteContentEditor
-                  ref={this.saveEditorRef}
-                  detectLanguage={detectLanguage}
-                  spellCheckEnabled={spellCheckEnabled}
                   storeFocusEditor={this.storeFocusContentEditor}
                   storeHasFocus={this.storeEditorHasFocus}
                   noteId={get(note, 'id', null)}
@@ -240,16 +225,10 @@ export class NoteDetail extends Component<Props> {
   }
 }
 
-const mapStateToProps: S.MapState<StateProps> = ({
-  appState: state,
-  ui,
-  settings,
-}) => ({
-  detectLanguage: settings.languageDetectionEnabled,
+const mapStateToProps: S.MapState<StateProps> = ({ appState: state, ui }) => ({
   dialogs: state.dialogs,
   note: ui.selectedRevision || ui.note,
   showNoteInfo: ui.showNoteInfo,
-  spellCheckEnabled: settings.spellCheckEnabled,
 });
 
 export default connect(mapStateToProps)(NoteDetail);