fix: load unsave handlers only after editors mount (#351)

src/renderer/app.tsx

@@ -78,7 +78,6 @@ export class App {
 
     // once loaded, we have a "saved" state
     this.state.isUnsaved = false;
-    this.setupUnsavedOnChangeListener();
 
     return true;
   }
@@ -177,30 +176,9 @@ export class App {
 
     ipcRenderer.send(WEBCONTENTS_READY_FOR_IPC_SIGNAL);
 
-    // TODO: A timer here is terrible. Let's fix this
-    // and ensure we actually do it once Editors have mounted.
-    setTimeout(() => {
-      this.setupUnsavedOnChangeListener();
-    }, 1500);
-
     return rendered;
   }
 
-  /**
-   * If the editor is changed for the first time, we'll
-   * set `isUnsaved` to true. That way, the app can warn you
-   * if you're about to throw things away.
-   */
-  public setupUnsavedOnChangeListener() {
-    Object.keys(window.ElectronFiddle.editors).forEach((key) => {
-      const editor = window.ElectronFiddle.editors[key];
-      const disposable = editor.onDidChangeModelContent(() => {
-        this.state.isUnsaved = true;
-        disposable.dispose();
-      });
-    });
-  }
-
   /**
    * Loads theme CSS into the HTML document.
    *

src/renderer/components/editors.tsx

@@ -66,16 +66,14 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
     this.setFocused = this.setFocused.bind(this);
 
     this.state = { monacoOptions: defaultMonacoOptions };
-
-    this.loadMonaco();
   }
 
   /**
    * Executed right after the component mounts. We'll setup the IPC listeners here.
    *
    * @memberof Editors
    */
-  public componentDidMount() {
+  public async componentDidMount() {
     ipcRendererManager.on(IpcEvents.MONACO_EXECUTE_COMMAND, (_event, cmd: string) => {
       this.executeCommand(cmd);
     });
@@ -95,6 +93,8 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
     });
 
     this.setState({ isMounted: true });
+    await this.loadMonaco();
+    this.props.appState.isUnsaved = false;
   }
 
   public componentWillUnmount() {
@@ -291,7 +291,7 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
    * We're doing things a bit roundabout to ensure that we're not overloading the
    * mobx state with a gigantic Monaco tree.
    */
-  public async loadMonaco(): Promise<void> {
+  public async loadMonaco() {
     const { app } = window.ElectronFiddle;
     const loader = require('monaco-loader');
     const monaco = app.monaco || await loader();
@@ -309,7 +309,7 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
       this.setState({ monaco });
     }
 
-    activateTheme(monaco, undefined, this.props.appState.theme);
+    await activateTheme(monaco, undefined, this.props.appState.theme);
   }
 
   /**

src/renderer/file-manager.ts

@@ -110,7 +110,6 @@ export class FileManager {
       }
 
       this.appState.isUnsaved = false;
-      window.ElectronFiddle.app.setupUnsavedOnChangeListener();
     }
   }
 

src/renderer/state.ts

@@ -104,7 +104,7 @@ export class AppState {
 
   @observable public isPublishing: boolean = false;
   @observable public isRunning: boolean = false;
-  @observable public isUnsaved: boolean = false;
+  @observable public isUnsaved: boolean;
   @observable public isUpdatingElectronVersions: boolean = false;
 
   // -- Various "isShowing" settings ------------------
@@ -167,7 +167,9 @@ export class AppState {
     autorun(() => this.save('statesToShow', this.statesToShow));
 
     autorun(() => {
-      if (this.isUnsaved) {
+      if (typeof this.isUnsaved === 'undefined') return;
+
+      if (!!this.isUnsaved) {
         window.onbeforeunload = () => {
           ipcRendererManager.send(IpcEvents.SHOW_INACTIVE);
           this.setGenericDialogOptions({
@@ -200,6 +202,15 @@ export class AppState {
         };
       } else {
         window.onbeforeunload = null;
+
+        // set up editor listeners to verify if unsaved
+        Object.keys(window.ElectronFiddle.editors).forEach((key) => {
+          const editor = window.ElectronFiddle.editors[key];
+          const disposable = editor.onDidChangeModelContent(() => {
+            this.isUnsaved = true;
+            disposable.dispose();
+          });
+        });
       }
     });
 

tests/renderer/app-spec.tsx

@@ -40,11 +40,9 @@ describe('Editors component', () => {
 
       const app = new App();
       const result = (await app.setup()) as HTMLDivElement;
-      app.setupUnsavedOnChangeListener = jest.fn();
       jest.runAllTimers();
 
       expect(result.innerHTML).toBe('Dialogs;Header;OutputEditorsWrapper;');
-      expect(app.setupUnsavedOnChangeListener).toHaveBeenCalled();
 
       jest.useRealTimers();
     });
@@ -174,7 +172,6 @@ describe('Editors component', () => {
       const app = new App();
       (app.state as Partial<AppState>) = new MockState();
       app.state.isUnsaved = false;
-      app.setupUnsavedOnChangeListener = jest.fn();
       app.setEditorValues = jest.fn();
 
       const editorValues = {
@@ -190,7 +187,6 @@ describe('Editors component', () => {
       })
         .then(() => {
           expect(app.state.isUnsaved).toBe(false);
-          expect(app.setupUnsavedOnChangeListener).toHaveBeenCalled();
           done();
         });
     });
@@ -336,23 +332,6 @@ describe('Editors component', () => {
     });
   });
 
-  describe('setupUnsavedOnChangeListener()', () => {
-    it('listens for model change events', async () => {
-      const app = new App();
-
-      app.setupUnsavedOnChangeListener();
-
-      const fn = window.ElectronFiddle.editors!.renderer!
-        .onDidChangeModelContent;
-      const call = (fn as jest.Mock<any>).mock.calls[0];
-      const cb = call[0];
-
-      cb();
-
-      expect(app.state.isUnsaved).toBe(true);
-    });
-  });
-
   describe('setupResizeListener()', () => {
     it('attaches to the handler', () => {
       window.addEventListener = jest.fn();

tests/renderer/state-spec.ts

@@ -55,8 +55,8 @@ describe('AppState', () => {
     expect(appState).toBeTruthy();
   });
 
-  describe('onbeforeunload handler', () => {
-    it('closes the window', (done) => {
+  describe('isUnsaved autorun handler', () => {
+    it('can close the window if user accepts the dialog', (done) => {
       window.close = jest.fn();
       appState.isUnsaved = true;
       expect(window.onbeforeunload).toBeTruthy();
@@ -73,7 +73,7 @@ describe('AppState', () => {
       });
     });
 
-    it('closes the app', (done) => {
+    it('can close the app after user accepts dialog', (done) => {
       const { remote } = require('electron');
       window.close = jest.fn();
       appState.isUnsaved = true;
@@ -93,7 +93,7 @@ describe('AppState', () => {
       });
     });
 
-    it('does not close the window', (done) => {
+    it('takes no action if user cancels the dialog', (done) => {
       window.close = jest.fn();
       appState.isUnsaved = true;
       expect(window.onbeforeunload).toBeTruthy();
@@ -109,6 +109,21 @@ describe('AppState', () => {
         done();
       });
     });
+
+    it('sets the onDidChangeModelContent handler if saved', () => {
+      appState.isUnsaved = false;
+
+      expect(window.onbeforeunload).toBe(null);
+
+      const fn = window.ElectronFiddle.editors!.renderer!
+        .onDidChangeModelContent;
+      const call = (fn as jest.Mock<any>).mock.calls[0];
+      const cb = call[0];
+
+      cb();
+
+      expect(appState.isUnsaved).toBe(true);
+    });
   });
 
   describe('updateElectronVersions()', () => {