Initialize path before checking Node installation (#300)

* Initialize path before checking Node installation * Address feedback * Quick-fix for initializePath (#305) * added resolve & modified throw location * Changed to have consistent resolve Co-Authored-By: AWolf81 <[email protected]> * convert stdout to utf8 string * Switch to promises to avoid weird issue * remove console.log
joshwcomeau · Oct 30, 2018 · 276d181 · 276d181
1 parent 8b78fc3
commit 276d181
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 42 deletions.
diff --git a/src/components/Initialization/Initialization.js b/src/components/Initialization/Initialization.js
@@ -25,26 +25,40 @@ class Initialization extends PureComponent<Props, State> {
     wasSuccessfullyInitialized: false,
   };
 
-  async componentDidMount() {
-    const nodeVersion = await getNodeJsVersion();
-
-    if (!nodeVersion) {
-      dialog.showErrorBox(
-        'Node missing',
-        'It looks like Node.js isn\'t installed. Node is required to use Guppy.\nWhen you click "OK", you\'ll be directed to instructions to download and install Node.'
-      );
-      shell.openExternal(
-        `${GUPPY_REPO_URL}/blob/master/README.md#installation`
-      );
-    }
-
-    initializePath();
-    this.setState({ wasSuccessfullyInitialized: !!nodeVersion });
-    logger.logEvent('load-application', {
-      node_version: nodeVersion,
-    });
-
-    ipcRenderer.on('app-will-close', this.appWillClose);
+  componentDidMount() {
+    initializePath()
+      .then(getNodeJsVersion)
+      .then(nodeVersion => {
+        this.setState({ wasSuccessfullyInitialized: !!nodeVersion });
+
+        logger.logEvent('load-application', {
+          node_version: nodeVersion,
+        });
+
+        if (!nodeVersion) {
+          throw new Error('node-not-found');
+        }
+
+        ipcRenderer.on('app-will-close', this.appWillClose);
+      })
+      .catch(e => {
+        switch (e.message) {
+          case 'node-not-found': {
+            dialog.showErrorBox(
+              'Node missing',
+              'It looks like Node.js isn\'t installed. Node is required to use Guppy.\nWhen you click "OK", you\'ll be directed to instructions to download and install Node.'
+            );
+            shell.openExternal(
+              `${GUPPY_REPO_URL}/blob/master/README.md#installation`
+            );
+            break;
+          }
+          default: {
+            // Path initialization can reject if no valid Node version is found.
+            // This isn't really an error, though, so we can swallow it.
+          }
+        }
+      });
   }
 
   appWillClose = () => {

diff --git a/src/services/platform.service.js b/src/services/platform.service.js
@@ -72,33 +72,46 @@ export const getBaseProjectEnvironment = (
   };
 };
 
-window.childProcess = childProcess;
-
 // HACK: With electron-builder, we're having some issues on mac finding Node.
 // This is because for some reason, the PATH is not updated properly :(
 // 'fix-path' is supposed to do this for us, but it doesn't work, for unknown
 // reasons.
 export const initializePath = () => {
-  childProcess.exec('which node', { env: window.process.env }, (_, version) => {
-    if (!version && isMac) {
-      // For users with a standard Node installation, node will be in
-      // /usr/local/bin
-      // For users using NVM, the path to Node will be added to `.bashrc`.
-      // Add both to the PATH.
-      try {
-        childProcess.exec(
-          'source ~/.bashrc && echo $PATH',
-          (err, updatedPath) => {
-            if (updatedPath) {
-              window.process.env.PATH = `/usr/local/bin:${updatedPath}`;
+  return new Promise(resolve => {
+    if (!isMac) {
+      return resolve();
+    }
+
+    // Check if we need to fix the Path (Mac only)
+    childProcess.exec(
+      'which node',
+      { env: window.process.env },
+      (_, nodePath) => {
+        if (nodePath) {
+          // Node found
+          return resolve();
+        }
+
+        // For users with a standard Node installation, node will be in
+        // /usr/local/bin
+        // For users using NVM, the path to Node will be added to `.bashrc`.
+        // Add both to the PATH.
+        try {
+          childProcess.exec(
+            'source ~/.bashrc && echo $PATH',
+            (err, updatedPath) => {
+              if (updatedPath) {
+                window.process.env.PATH = `/usr/local/bin:${updatedPath}`;
+              }
+
+              resolve();
             }
-          }
-        );
-      } catch (e) {
-        // If no `.bashrc` exists, we have no work to do.
-        // The PATH should already be set correctly.
+          );
+        } catch (e) {
+          resolve(e);
+        }
       }
-    }
+    );
   });
 };
 

diff --git a/src/services/shell.service.js b/src/services/shell.service.js
@@ -16,11 +16,11 @@ export const openProjectInEditor = (project: Project) =>
 
 export const getNodeJsVersion = () =>
   new Promise(resolve =>
-    exec('node -v', (error, stdout) => {
+    exec('node -v', { env: window.process.env }, (error, stdout) => {
       if (error) {
         return resolve();
       }
 
-      resolve(stdout.trim());
+      resolve(stdout.toString().trim());
     })
   );