From bb23b360515a780c6f58c9e1919033e65cb01f40 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Fri, 10 Dec 2021 17:58:06 -0800
Subject: [PATCH] Fixed flipper settings not being loaded / saved on fresh
 install

Summary:
I was running flipper-server on a fresh machine without Flipper installed and discovered that reading / writing settings failed since `~/.flipper` wasn't existing, due to using `access` instead of `pathExists`.

Added a warning about needing to restart the server after making changes, since that is tricky to do from the UI.

Fixed an issue in the settings screen, which would fs.stat as part of rendering, causing the Settings UI not to load.

Reviewed By: timur-valiev, aigoncharov

Differential Revision: D32984538

fbshipit-source-id: 2b2011ad9d84c72ac824d92a8c96f636237b8771
---
 .../src/utils/settings.tsx                    | 21 +++++++++++++------
 .../src/initializeRenderHost.tsx              |  5 +++--
 .../src/chrome/settings/configFields.tsx      | 18 +++++++++-------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/desktop/flipper-server-core/src/utils/settings.tsx b/desktop/flipper-server-core/src/utils/settings.tsx
index 37c90ca68..1fa647143 100644
--- a/desktop/flipper-server-core/src/utils/settings.tsx
+++ b/desktop/flipper-server-core/src/utils/settings.tsx
@@ -11,30 +11,39 @@ import os from 'os';
 import {resolve} from 'path';
 import xdg from 'xdg-basedir';
 import {Settings, Tristate} from 'flipper-common';
-import {readFile, writeFile, access} from 'fs-extra';
+import {readFile, writeFile, pathExists, mkdirp} from 'fs-extra';
 
 export async function loadSettings(): Promise<Settings> {
-  if (!access(getSettingsFile())) {
+  if (!pathExists(getSettingsFile())) {
+    return getDefaultSettings();
+  }
+  try {
+    const json = await readFile(getSettingsFile(), {encoding: 'utf8'});
+    return JSON.parse(json);
+  } catch (e) {
+    console.warn('Failed to load settings file', e);
     return getDefaultSettings();
   }
-  const json = await readFile(getSettingsFile(), {encoding: 'utf8'});
-  return JSON.parse(json);
 }
 
 export async function saveSettings(settings: Settings): Promise<void> {
+  await mkdirp(getSettingsDir());
   await writeFile(getSettingsFile(), JSON.stringify(settings, null, 2), {
     encoding: 'utf8',
   });
 }
 
-function getSettingsFile() {
+function getSettingsDir() {
   return resolve(
     ...(xdg.config ? [xdg.config] : [os.homedir(), '.config']),
     'flipper',
-    'settings.json',
   );
 }
 
+function getSettingsFile() {
+  return resolve(getSettingsDir(), 'settings.json');
+}
+
 export const DEFAULT_ANDROID_SDK_PATH = getDefaultAndroidSdkPath();
 
 function getDefaultSettings(): Settings {
diff --git a/desktop/flipper-ui-browser/src/initializeRenderHost.tsx b/desktop/flipper-ui-browser/src/initializeRenderHost.tsx
index 816aa95e3..f04f9a346 100644
--- a/desktop/flipper-ui-browser/src/initializeRenderHost.tsx
+++ b/desktop/flipper-ui-browser/src/initializeRenderHost.tsx
@@ -52,8 +52,9 @@ export function initializeRenderHost(
       );
     },
     restartFlipper() {
-      // TODO: restart server as well
-      window.location.reload();
+      window.flipperShowError!(
+        'Flipper settings have changed, please restart flipper server for the changes to take effect',
+      );
     },
     loadDefaultPlugins: getDefaultPluginsIndex,
     serverConfig: flipperServerConfig,
diff --git a/desktop/flipper-ui-core/src/chrome/settings/configFields.tsx b/desktop/flipper-ui-core/src/chrome/settings/configFields.tsx
index 7108689b1..868095cb1 100644
--- a/desktop/flipper-ui-core/src/chrome/settings/configFields.tsx
+++ b/desktop/flipper-ui-core/src/chrome/settings/configFields.tsx
@@ -20,6 +20,7 @@ import React, {useState} from 'react';
 import {promises as fs} from 'fs';
 import {theme} from 'flipper-plugin';
 import {getRenderHostInstance} from '../../RenderHost';
+import {useEffect} from 'react';
 
 export const ConfigFieldContainer = styled(FlexRow)({
   paddingLeft: 10,
@@ -70,14 +71,17 @@ export function FilePathConfigField(props: {
   const renderHost = getRenderHostInstance();
   const [value, setValue] = useState(props.defaultValue);
   const [isValid, setIsValid] = useState(true);
-  fs.stat(value)
-    .then((stat) => props.isRegularFile !== stat.isDirectory())
-    .then((valid) => {
-      if (valid !== isValid) {
-        setIsValid(valid);
+
+  useEffect(() => {
+    (async function () {
+      try {
+        const stat = await fs.stat(value);
+        setIsValid(props.isRegularFile !== stat.isDirectory());
+      } catch (_) {
+        setIsValid(false);
       }
-    })
-    .catch((_) => setIsValid(false));
+    })();
+  }, [props.isRegularFile, value]);
 
   return (
     <ConfigFieldContainer>