Convert the FilePatchController hierarchy to React

lib/controllers/file-patch-controller.js

@@ -1,41 +1,47 @@
-/** @jsx etch.dom */
-/* eslint react/no-unknown-property: "off" */
+import React from 'react';
 
-import {Emitter, Point} from 'atom';
-import etch from 'etch';
+import {Point, Emitter} from 'atom';
 import {autobind} from 'core-decorators';
 
+import EventWatcher from '../event-watcher';
 import FilePatchView from '../views/file-patch-view';
 
-export default class FilePatchController {
-  constructor(props) {
-    this.props = props;
-    this.emitter = new Emitter();
-    this.stagingOperationInProgress = false;
-    etch.initialize(this);
+export default class FilePatchController extends React.Component {
+  static propTypes = {
+    repository: React.PropTypes.object,
+    commandRegistry: React.PropTypes.object.isRequired,
+    filePatch: React.PropTypes.object.isRequired,
+    stagingStatus: React.PropTypes.oneOf(['unstaged', 'staged']).isRequired,
+    isPartiallyStaged: React.PropTypes.bool.isRequired,
+    isAmending: React.PropTypes.bool.isRequired,
+    discardLines: React.PropTypes.func.isRequired,
+    didSurfaceFile: React.PropTypes.func.isRequired,
+    didDiveIntoFilePath: React.PropTypes.func.isRequired,
+    quietlySelectItem: React.PropTypes.func.isRequired,
+    undoLastDiscard: React.PropTypes.func.isRequired,
+    openFiles: React.PropTypes.func.isRequired,
+    eventWatcher: React.PropTypes.instanceOf(EventWatcher),
   }
 
-  serialize() {
-    return null;
+  static defaultProps = {
+    eventWatcher: new EventWatcher(),
   }
 
-  update(props) {
-    // If we update the active repository to null and that gets passed to this
-    // component, we'll instead hold on to our current repository (since it will
-    // not change for a given file).
-    const repository = props.repository || this.props.repository;
-    this.props = {...this.props, ...props, repository};
-    this.emitter.emit('did-change-title', this.getTitle());
-    return etch.update(this);
+  constructor(props, context) {
+    super(props, context);
+
+    this.stagingOperationInProgress = false;
+    this.emitter = new Emitter();
   }
 
-  destroy() {
-    this.emitter.emit('did-destroy');
-    return etch.destroy(this);
+  serialize() {
+    return null;
   }
 
-  getRepository() {
-    return this.props.repository;
+  componentWillReceiveProps(nextProps) {
+    if (this.getTitle(nextProps) !== this.getTitle()) {
+      this.emitter.emit('did-change-title', this.getTitle(nextProps));
+    }
   }
 
   render() {
@@ -53,17 +59,16 @@ export default class FilePatchController {
       return (
         <div className="github-PaneView pane-item">
           <FilePatchView
-            ref="filePatchView"
             commandRegistry={this.props.commandRegistry}
-            attemptLineStageOperation={this.attemptLineStageOperation}
-            didSurfaceFile={this.didSurfaceFile}
-            didDiveIntoCorrespondingFilePatch={this.diveIntoCorrespondingFilePatch}
-            attemptHunkStageOperation={this.attemptHunkStageOperation}
             hunks={hunks}
             filePath={filePath}
             stagingStatus={this.props.stagingStatus}
             isPartiallyStaged={this.props.isPartiallyStaged}
-            registerHunkView={this.props.registerHunkView}
+            attemptLineStageOperation={this.attemptLineStageOperation}
+            attemptHunkStageOperation={this.attemptHunkStageOperation}
+            didSurfaceFile={this.didSurfaceFile}
+            didDiveIntoCorrespondingFilePatch={this.diveIntoCorrespondingFilePatch}
+            eventWatcher={this.props.eventWatcher}
             openCurrentFile={this.openCurrentFile}
             discardLines={this.props.discardLines}
             undoLastDiscard={this.undoLastDiscard}
@@ -74,16 +79,26 @@ export default class FilePatchController {
     }
   }
 
-  stageHunk(hunk) {
-    return this.props.repository.applyPatchToIndex(
+  onDidChangeTitle(callback) {
+    return this.emitter.on('did-change-title', callback);
+  }
+
+  onDidDestroy(callback) {
+    return this.emitter.on('did-destroy', callback);
+  }
+
+  async stageHunk(hunk) {
+    await this.props.repository.applyPatchToIndex(
       this.props.filePatch.getStagePatchForHunk(hunk),
     );
+    this.props.eventWatcher.resolveStageOperationPromise();
   }
 
   async unstageHunk(hunk) {
     await this.props.repository.applyPatchToIndex(
       this.props.filePatch.getUnstagePatchForHunk(hunk),
     );
+    this.props.eventWatcher.resolveStageOperationPromise();
   }
 
   stageOrUnstageHunk(hunk) {
@@ -99,33 +114,31 @@ export default class FilePatchController {
   @autobind
   attemptHunkStageOperation(hunk) {
     if (this.stagingOperationInProgress) {
-      return {
-        stageOperationPromise: Promise.resolve(),
-        selectionUpdatePromise: Promise.resolve(),
-      };
+      return;
     }
 
     this.stagingOperationInProgress = true;
-
-    const hunkUpdatePromise = this.refs.filePatchView.getNextHunkUpdatePromise();
-    const stageOperationPromise = this.stageOrUnstageHunk(hunk);
-    const selectionUpdatePromise = hunkUpdatePromise.then(() => {
+    this.props.eventWatcher.getPatchChangedPromise().then(() => {
       this.stagingOperationInProgress = false;
     });
 
-    return {stageOperationPromise, selectionUpdatePromise};
+    this.stageOrUnstageHunk(hunk);
   }
 
-  stageLines(lines) {
-    return this.props.repository.applyPatchToIndex(
+  async stageLines(lines) {
+    await this.props.repository.applyPatchToIndex(
       this.props.filePatch.getStagePatchForLines(lines),
     );
+
+    this.props.eventWatcher.resolveStageOperationPromise();
   }
 
   async unstageLines(lines) {
     await this.props.repository.applyPatchToIndex(
       this.props.filePatch.getUnstagePatchForLines(lines),
     );
+
+    this.props.eventWatcher.resolveStageOperationPromise();
   }
 
   stageOrUnstageLines(lines) {
@@ -141,38 +154,24 @@ export default class FilePatchController {
   @autobind
   attemptLineStageOperation(lines) {
     if (this.stagingOperationInProgress) {
-      return {
-        stageOperationPromise: Promise.resolve(),
-        selectionUpdatePromise: Promise.resolve(),
-      };
+      return;
     }
 
     this.stagingOperationInProgress = true;
-
-    const hunkUpdatePromise = this.refs.filePatchView.getNextHunkUpdatePromise();
-    const stageOperationPromise = this.stageOrUnstageLines(lines);
-    const selectionUpdatePromise = hunkUpdatePromise.then(() => {
+    this.props.eventWatcher.getPatchChangedPromise().then(() => {
       this.stagingOperationInProgress = false;
     });
 
-    return {stageOperationPromise, selectionUpdatePromise};
+    this.stageOrUnstageLines(lines);
   }
 
-  getTitle() {
-    let title = this.props.stagingStatus === 'staged' ? 'Staged' : 'Unstaged';
+  getTitle(props = this.props) {
+    let title = props.stagingStatus === 'staged' ? 'Staged' : 'Unstaged';
     title += ' Changes: ';
-    title += this.props.filePatch.getPath();
+    title += props.filePatch.getPath();
     return title;
   }
 
-  onDidChangeTitle(callback) {
-    return this.emitter.on('did-change-title', callback);
-  }
-
-  onDidDestroy(callback) {
-    return this.emitter.on('did-destroy', callback);
-  }
-
   @autobind
   didSurfaceFile() {
     if (this.props.didSurfaceFile) {
@@ -188,15 +187,6 @@ export default class FilePatchController {
     return this.props.didDiveIntoFilePath(filePath, stagingStatus, {amending: this.props.isAmending});
   }
 
-  didUpdateFilePatch() {
-    // FilePatch was mutated so all we need to do is re-render
-    return etch.update(this);
-  }
-
-  didDestroyFilePatch() {
-    this.destroy();
-  }
-
   focus() {
     if (this.refs.filePatchView) {
       this.refs.filePatchView.focus();
@@ -221,4 +211,8 @@ export default class FilePatchController {
   hasUndoHistory() {
     return this.props.repository.hasDiscardHistory(this.props.filePatch.getPath());
   }
+
+  destroy() {
+    this.emitter.emit('did-destroy');
+  }
 }

lib/controllers/root-controller.js

@@ -29,8 +29,8 @@ import {GitError} from '../git-shell-out-strategy';
 const nullFilePatchState = {
   filePath: null,
   filePatch: null,
-  stagingStatus: null,
-  partiallyStaged: null,
+  stagingStatus: 'unstaged',
+  partiallyStaged: false,
 };
 
 export default class RootController extends React.Component {
@@ -195,26 +195,23 @@ export default class RootController extends React.Component {
         </Commands>
         <PaneItem
           workspace={this.props.workspace}
-          getItem={({subtree}) => subtree.getWrappedComponent()}
           ref={c => { this.filePatchControllerPane = c; }}
           onDidCloseItem={() => { this.setState({...nullFilePatchState}); }}>
-          <EtchWrapper ref={c => { this.filePatchController = c; }} reattachDomNode={false}>
-            <FilePatchController
-              repository={this.props.repository}
-              commandRegistry={this.props.commandRegistry}
-              filePatch={this.state.filePatch}
-              stagingStatus={this.state.stagingStatus}
-              isAmending={this.state.amending}
-              isPartiallyStaged={this.state.partiallyStaged}
-              onRepoRefresh={this.onRepoRefresh}
-              didSurfaceFile={this.surfaceFromFileAtPath}
-              didDiveIntoFilePath={this.diveIntoFilePatchForPath}
-              quietlySelectItem={this.quietlySelectItem}
-              openFiles={this.openFiles}
-              discardLines={this.discardLines}
-              undoLastDiscard={this.undoLastDiscard}
-            />
-          </EtchWrapper>
+          <FilePatchController
+            repository={this.props.repository}
+            commandRegistry={this.props.commandRegistry}
+            filePatch={this.state.filePatch}
+            stagingStatus={this.state.stagingStatus}
+            isAmending={this.state.amending}
+            isPartiallyStaged={this.state.partiallyStaged}
+            onRepoRefresh={this.onRepoRefresh}
+            didSurfaceFile={this.surfaceFromFileAtPath}
+            didDiveIntoFilePath={this.diveIntoFilePatchForPath}
+            quietlySelectItem={this.quietlySelectItem}
+            openFiles={this.openFiles}
+            discardLines={this.discardLines}
+            undoLastDiscard={this.undoLastDiscard}
+          />
         </PaneItem>
       </div>
     );
@@ -436,7 +433,9 @@ export default class RootController extends React.Component {
 
   @autobind
   focusFilePatchView() {
-    this.filePatchController.getWrappedComponent().focus();
+    const item = this.filePatchControllerPane.getPaneItem();
+    const viewElement = item.getElement().querySelector('[tabindex]');
+    viewElement.focus();
   }
 
   @autobind

lib/event-watcher.js

@@ -0,0 +1,87 @@
+/*
+ * Construct Promises to wait for the next occurrence of specific events that occur throughout the data refresh
+ * and rendering cycle. Resolve those promises when the corresponding events have been observed.
+ */
+export default class EventWatcher {
+  constructor() {
+    this.promises = new Map();
+  }
+
+  /*
+   * Retrieve a Promise that will be resolved the next time a desired event is observed.
+   *
+   * In general, you should prefer the more specific `getXyzPromise()` methods instead to avoid a proliferation of
+   * "magic strings."
+   */
+  getPromise(eventName) {
+    const existing = this.promises.get(eventName);
+    if (existing !== undefined) {
+      return existing.promise;
+    }
+
+    let resolver, rejecter;
+    const created = new Promise((resolve, reject) => {
+      resolver = resolve;
+      rejecter = reject;
+    });
+    this.promises.set(eventName, {
+      promise: created,
+      resolver,
+      rejecter,
+    });
+    return created;
+  }
+
+  /*
+   * Indicate that a named event has been observed, resolving any Promises that were created for this event. Optionally
+   * provide a payload.
+   *
+   * In general, you should prefer the more specific `resolveXyzPromise()` methods.
+   */
+  resolvePromise(eventName, payload) {
+    const existing = this.promises.get(eventName);
+    if (existing !== undefined) {
+      this.promises.delete(eventName);
+      existing.resolver(payload);
+    }
+  }
+
+  /*
+   * Indicate that a named event has had some kind of terrible problem.
+   */
+  rejectPromise(eventName, error) {
+    const existing = this.promises.get(eventName);
+    if (existing !== undefined) {
+      this.promises.delete(eventName);
+      existing.rejecter(error);
+    }
+  }
+
+  /*
+   * Notified when a hunk or line stage or unstage operation has completed.
+   */
+  getStageOperationPromise() {
+    return this.getPromise('stage-operation');
+  }
+
+  /*
+   * Notified when an open FilePatchView's hunks have changed.
+   */
+  getPatchChangedPromise() {
+    return this.getPromise('patch-changed');
+  }
+
+  /*
+   * A hunk or line stage or unstage operation has completed.
+   */
+  resolveStageOperationPromise(payload) {
+    this.resolvePromise('stage-operation', payload);
+  }
+
+  /*
+   * An open FilePatchView's hunks have changed.
+   */
+  resolvePatchChangedPromise(payload) {
+    this.resolvePromise('patch-changed', payload);
+  }
+}