Live reload on file tree changes and project settings changes (#4142)

* Reload FileTree and File when changed externally

* Added tests

* Make file and project creation a bit more reliable

e2e/playwright/code-pane-and-errors.spec.ts

@@ -313,3 +313,45 @@ test(
     await electronApp.close()
   }
 )
+
+test(
+  'external change of file contents are reflected in editor',
+  { tag: '@electron' },
+  async ({ browserName }, testInfo) => {
+    const PROJECT_DIR_NAME = 'lee-was-here'
+    const {
+      electronApp,
+      page,
+      dir: projectsDir,
+    } = await setupElectron({
+      testInfo,
+      folderSetupFn: async (dir) => {
+        const aProjectDir = join(dir, PROJECT_DIR_NAME)
+        await fsp.mkdir(aProjectDir, { recursive: true })
+      },
+    })
+
+    const u = await getUtils(page)
+    await page.setViewportSize({ width: 1200, height: 500 })
+
+    await test.step('Open the project', async () => {
+      await expect(page.getByText(PROJECT_DIR_NAME)).toBeVisible()
+      await page.getByText(PROJECT_DIR_NAME).click()
+      await u.waitForPageLoad()
+    })
+
+    await u.openFilePanel()
+    await u.openKclCodePanel()
+
+    await test.step('Write to file externally and check for changed content', async () => {
+      const content = 'ha he ho ho ha blap scap be dap'
+      await fsp.writeFile(
+        join(projectsDir, PROJECT_DIR_NAME, 'main.kcl'),
+        content
+      )
+      await u.editorTextMatches(content)
+    })
+
+    await electronApp.close()
+  }
+)

e2e/playwright/file-tree.spec.ts

@@ -960,4 +960,171 @@ _test.describe('Deleting items from the file pane', () => {
     'TODO - delete folder we are in, with no main.kcl',
     async () => {}
   )
+
+  // Copied from tests above.
+  _test(
+    `external deletion of project navigates back home`,
+    { tag: '@electron' },
+    async ({ browserName }, testInfo) => {
+      const TEST_PROJECT_NAME = 'Test Project'
+      const {
+        electronApp,
+        page,
+        dir: projectsDirName,
+      } = await setupElectron({
+        testInfo,
+        folderSetupFn: async (dir) => {
+          await fsp.mkdir(join(dir, TEST_PROJECT_NAME), { recursive: true })
+          await fsp.mkdir(join(dir, TEST_PROJECT_NAME, 'folderToDelete'), {
+            recursive: true,
+          })
+          await fsp.copyFile(
+            executorInputPath('basic_fillet_cube_end.kcl'),
+            join(dir, TEST_PROJECT_NAME, 'main.kcl')
+          )
+          await fsp.copyFile(
+            executorInputPath('cylinder.kcl'),
+            join(dir, TEST_PROJECT_NAME, 'folderToDelete', 'someFileWithin.kcl')
+          )
+        },
+      })
+      const u = await getUtils(page)
+      await page.setViewportSize({ width: 1200, height: 500 })
+
+      // Constants and locators
+      const projectCard = page.getByText(TEST_PROJECT_NAME)
+      const projectMenuButton = page.getByTestId('project-sidebar-toggle')
+      const folderToDelete = page.getByRole('button', {
+        name: 'folderToDelete',
+      })
+      const fileWithinFolder = page.getByRole('listitem').filter({
+        has: page.getByRole('button', { name: 'someFileWithin.kcl' }),
+      })
+
+      await _test.step(
+        'Open project and navigate into folderToDelete',
+        async () => {
+          await projectCard.click()
+          await u.waitForPageLoad()
+          await _expect(projectMenuButton).toContainText('main.kcl')
+          await u.closeKclCodePanel()
+          await u.openFilePanel()
+
+          await folderToDelete.click()
+          await _expect(fileWithinFolder).toBeVisible()
+          await fileWithinFolder.click()
+          await _expect(projectMenuButton).toContainText('someFileWithin.kcl')
+        }
+      )
+
+      // Point of divergence. Delete the project folder and see if it goes back
+      // to the home view.
+      await _test.step(
+        'Delete projectsDirName/<project-name> externally',
+        async () => {
+          await fsp.rm(join(projectsDirName, TEST_PROJECT_NAME), {
+            recursive: true,
+            force: true,
+          })
+        }
+      )
+
+      await _test.step('Check the app is back on the home view', async () => {
+        const projectsDirLink = page.getByText('Loaded from')
+        await _expect(projectsDirLink).toBeVisible()
+      })
+
+      await electronApp.close()
+    }
+  )
+
+  // Similar to the above
+  _test(
+    `external deletion of file in sub-directory updates the file tree and recreates it on code editor typing`,
+    { tag: '@electron' },
+    async ({ browserName }, testInfo) => {
+      const TEST_PROJECT_NAME = 'Test Project'
+      const {
+        electronApp,
+        page,
+        dir: projectsDirName,
+      } = await setupElectron({
+        testInfo,
+        folderSetupFn: async (dir) => {
+          await fsp.mkdir(join(dir, TEST_PROJECT_NAME), { recursive: true })
+          await fsp.mkdir(join(dir, TEST_PROJECT_NAME, 'folderToDelete'), {
+            recursive: true,
+          })
+          await fsp.copyFile(
+            executorInputPath('basic_fillet_cube_end.kcl'),
+            join(dir, TEST_PROJECT_NAME, 'main.kcl')
+          )
+          await fsp.copyFile(
+            executorInputPath('cylinder.kcl'),
+            join(dir, TEST_PROJECT_NAME, 'folderToDelete', 'someFileWithin.kcl')
+          )
+        },
+      })
+      const u = await getUtils(page)
+      await page.setViewportSize({ width: 1200, height: 500 })
+
+      // Constants and locators
+      const projectCard = page.getByText(TEST_PROJECT_NAME)
+      const projectMenuButton = page.getByTestId('project-sidebar-toggle')
+      const folderToDelete = page.getByRole('button', {
+        name: 'folderToDelete',
+      })
+      const fileWithinFolder = page.getByRole('listitem').filter({
+        has: page.getByRole('button', { name: 'someFileWithin.kcl' }),
+      })
+
+      await _test.step(
+        'Open project and navigate into folderToDelete',
+        async () => {
+          await projectCard.click()
+          await u.waitForPageLoad()
+          await _expect(projectMenuButton).toContainText('main.kcl')
+
+          await u.openFilePanel()
+
+          await folderToDelete.click()
+          await _expect(fileWithinFolder).toBeVisible()
+          await fileWithinFolder.click()
+          await _expect(projectMenuButton).toContainText('someFileWithin.kcl')
+        }
+      )
+
+      await _test.step(
+        'Delete projectsDirName/<project-name> externally',
+        async () => {
+          await fsp.rm(
+            join(
+              projectsDirName,
+              TEST_PROJECT_NAME,
+              'folderToDelete',
+              'someFileWithin.kcl'
+            )
+          )
+        }
+      )
+
+      await _test.step('Check the file is gone in the file tree', async () => {
+        await _expect(
+          page.getByTestId('file-pane-scroll-container')
+        ).not.toContainText('someFileWithin.kcl')
+      })
+
+      await _test.step(
+        'Check the file is back in the file tree after typing in code editor',
+        async () => {
+          await u.pasteCodeInEditor('hello = 1')
+          await _expect(
+            page.getByTestId('file-pane-scroll-container')
+          ).toContainText('someFileWithin.kcl')
+        }
+      )
+
+      await electronApp.close()
+    }
+  )
 })

e2e/playwright/test-utils.ts

@@ -463,6 +463,9 @@ export async function getUtils(page: Page, test_?: typeof test) {
       return test_?.step(
         `Create and select project with text "${hasText}"`,
         async () => {
+          // Without this, we get unreliable project creation. It's probably
+          // due to a race between the FS being read and clicking doing something.
+          await page.waitForTimeout(100)
           await page.getByTestId('home-new-file').click()
           const projectLinksPost = page.getByTestId('project-link')
           await projectLinksPost.filter({ hasText }).click()
@@ -492,6 +495,11 @@ export async function getUtils(page: Page, test_?: typeof test) {
 
     createNewFile: async (name: string) => {
       return test?.step(`Create a file named ${name}`, async () => {
+        // If the application is in the middle of connecting a stream
+        // then creating a new file won't work in the end.
+        await expect(
+          page.getByRole('button', { name: 'Start Sketch' })
+        ).not.toBeDisabled()
         await page.getByTestId('create-file-button').click()
         await page.getByTestId('file-rename-field').fill(name)
         await page.keyboard.press('Enter')

e2e/playwright/testing-settings.spec.ts

@@ -9,7 +9,7 @@ import {
   executorInputPath,
 } from './test-utils'
 import { SaveSettingsPayload, SettingsLevel } from 'lib/settings/settingsTypes'
-import { SETTINGS_FILE_NAME } from 'lib/constants'
+import { SETTINGS_FILE_NAME, PROJECT_SETTINGS_FILE_NAME } from 'lib/constants'
 import {
   TEST_SETTINGS_KEY,
   TEST_SETTINGS_CORRUPTED,
@@ -445,6 +445,58 @@ test.describe('Testing settings', () => {
     }
   )
 
+  test(
+    'project settings reload on external change',
+    { tag: '@electron' },
+    async ({ browserName }, testInfo) => {
+      const {
+        electronApp,
+        page,
+        dir: projectDirName,
+      } = await setupElectron({
+        testInfo,
+      })
+
+      await page.setViewportSize({ width: 1200, height: 500 })
+
+      const logoLink = page.getByTestId('app-logo')
+      const projectDirLink = page.getByText('Loaded from')
+
+      await test.step('Wait for project view', async () => {
+        await expect(projectDirLink).toBeVisible()
+      })
+
+      const projectLinks = page.getByTestId('project-link')
+      const oldCount = await projectLinks.count()
+      await page.getByRole('button', { name: 'New project' }).click()
+      await expect(projectLinks).toHaveCount(oldCount + 1)
+      await projectLinks.filter({ hasText: 'project-000' }).first().click()
+
+      const changeColorFs = async (color: string) => {
+        const tempSettingsFilePath = join(
+          projectDirName,
+          'project-000',
+          PROJECT_SETTINGS_FILE_NAME
+        )
+        await fsp.writeFile(
+          tempSettingsFilePath,
+          `[settings.app]\nthemeColor = "${color}"`
+        )
+      }
+
+      await test.step('Check the color is first starting as we expect', async () => {
+        await expect(logoLink).toHaveCSS('--primary-hue', '264.5')
+      })
+
+      await test.step('Check color of logo changed', async () => {
+        await changeColorFs('99')
+        await expect(logoLink).toHaveCSS('--primary-hue', '99')
+      })
+
+      await electronApp.close()
+    }
+  )
+
   test(
     `Closing settings modal should go back to the original file being viewed`,
     { tag: '@electron' },

interface.d.ts

@@ -20,9 +20,10 @@ export interface IElectronAPI {
   version: typeof process.env.version
   watchFileOn: (
     path: string,
+    key: string,
     callback: (eventType: string, path: string) => void
   ) => void
-  watchFileOff: (path: string) => void
+  watchFileOff: (path: string, key: string) => void
   readFile: (path: string) => ReturnType<fs.readFile>
   writeFile: (
     path: string,

src/components/FileTree.tsx

@@ -2,7 +2,7 @@ import type { IndexLoaderData } from 'lib/types'
 import { PATHS } from 'lib/paths'
 import { ActionButton } from './ActionButton'
 import Tooltip from './Tooltip'
-import { Dispatch, useCallback, useEffect, useRef, useState } from 'react'
+import { Dispatch, useCallback, useRef, useState } from 'react'
 import { useNavigate, useRouteLoaderData } from 'react-router-dom'
 import { Disclosure } from '@headlessui/react'
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
@@ -13,7 +13,6 @@ import { sortProject } from 'lib/desktopFS'
 import { FILE_EXT } from 'lib/constants'
 import { CustomIcon } from './CustomIcon'
 import { codeManager, kclManager } from 'lib/singletons'
-import { useDocumentHasFocus } from 'hooks/useDocumentHasFocus'
 import { useLspContext } from './LspProvider'
 import useHotkeyWrapper from 'lib/hotkeyWrapper'
 import { useModelingContext } from 'hooks/useModelingContext'
@@ -21,6 +20,8 @@ import { DeleteConfirmationDialog } from './ProjectCard/DeleteProjectDialog'
 import { ContextMenu, ContextMenuItem } from './ContextMenu'
 import usePlatform from 'hooks/usePlatform'
 import { FileEntry } from 'lib/project'
+import { useFileSystemWatcher } from 'hooks/useFileSystemWatcher'
+import { normalizeLineEndings } from 'lib/codeEditor'
 
 function getIndentationCSS(level: number) {
   return `calc(1rem * ${level + 1})`
@@ -131,6 +132,23 @@ const FileTreeItem = ({
   const isCurrentFile = fileOrDir.path === currentFile?.path
   const itemRef = useRef(null)
 
+  // Since every file or directory gets its own FileTreeItem, we can do this.
+  // Because subtrees only render when they are opened, that means this
+  // only listens when they open. Because this acts like a useEffect, when
+  // the ReactNodes are destroyed, so is this listener :)
+  useFileSystemWatcher(
+    async (eventType, path) => {
+      // Don't try to read a file that was removed.
+      if (isCurrentFile && eventType !== 'unlink') {
+        let code = await window.electron.readFile(path)
+        code = normalizeLineEndings(code)
+        codeManager.updateCodeStateEditor(code)
+      }
+      fileSend({ type: 'Refresh' })
+    },
+    [fileOrDir.path]
+  )
+
   const isRenaming = fileContext.itemsBeingRenamed.includes(fileOrDir.path)
   const removeCurrentItemFromRenaming = useCallback(
     () =>
@@ -154,6 +172,13 @@ const FileTreeItem = ({
     })
   }, [fileContext.itemsBeingRenamed, fileOrDir.path, fileSend])
 
+  const clickDirectory = () => {
+    fileSend({
+      type: 'Set selected directory',
+      directory: fileOrDir,
+    })
+  }
+
   function handleKeyUp(e: React.KeyboardEvent<HTMLButtonElement>) {
     if (e.metaKey && e.key === 'Backspace') {
       // Open confirmation dialog
@@ -242,18 +267,8 @@ const FileTreeItem = ({
                   }
                   style={{ paddingInlineStart: getIndentationCSS(level) }}
                   onClick={(e) => e.currentTarget.focus()}
-                  onClickCapture={(e) =>
+                  onClickCapture={clickDirectory}
-                    fileSend({
+                  onFocusCapture={clickDirectory}
-                      type: 'Set selected directory',
-                      directory: fileOrDir,
-                    })
-                  }
-                  onFocusCapture={(e) =>
-                    fileSend({
-                      type: 'Set selected directory',
-                      directory: fileOrDir,
-                    })
-                  }
                   onKeyDown={(e) => e.key === 'Enter' && e.preventDefault()}
                   onKeyUp={handleKeyUp}
                 >
@@ -469,27 +484,30 @@ export const FileTreeInner = ({
   const loaderData = useRouteLoaderData(PATHS.FILE) as IndexLoaderData
   const { send: fileSend, context: fileContext } = useFileContext()
   const { send: modelingSend } = useModelingContext()
-  const documentHasFocus = useDocumentHasFocus()
 
-  // Refresh the file tree when the document gets focus
+  // Refresh the file tree when there are changes.
-  useEffect(() => {
+  useFileSystemWatcher(
-    fileSend({ type: 'Refresh' })
+    async (eventType, path) => {
-  }, [documentHasFocus])
+      fileSend({ type: 'Refresh' })
+    },
+    [loaderData?.project?.path, fileContext.selectedDirectory.path].filter(
+      (x: string | undefined) => x !== undefined
+    )
+  )
+
+  const clickDirectory = () => {
+    fileSend({
+      type: 'Set selected directory',
+      directory: fileContext.project,
+    })
+  }
 
   return (
     <div
       className="overflow-auto pb-12 absolute inset-0"
       data-testid="file-pane-scroll-container"
     >
-      <ul
+      <ul className="m-0 p-0 text-sm" onClickCapture={clickDirectory}>
-        className="m-0 p-0 text-sm"
-        onClickCapture={(e) => {
-          fileSend({
-            type: 'Set selected directory',
-            directory: fileContext.project,
-          })
-        }}
-      >
         {sortProject(fileContext.project?.children || []).map((fileOrDir) => (
           <FileTreeItem
             project={fileContext.project}

src/components/SettingsAuthProvider.tsx

@@ -221,6 +221,19 @@ export const SettingsAuthProviderBase = ({
 
   useFileSystemWatcher(
     async () => {
+      // If there is a projectPath but it no longer exists it means
+      // it was exterally removed. If we let the code past this condition
+      // execute it will recreate the directory due to code in
+      // loadAndValidateSettings trying to recreate files. I do not
+      // wish to change the behavior in case anything else uses it.
+      // Go home.
+      if (loadedProject?.project?.path) {
+        if (!window.electron.exists(loadedProject?.project?.path)) {
+          navigate(PATHS.HOME)
+          return
+        }
+      }
+
       const data = await loadAndValidateSettings(loadedProject?.project?.path)
       settingsSend({
         type: 'Set all settings',
@@ -228,7 +241,9 @@ export const SettingsAuthProviderBase = ({
         doNotPersist: true,
       })
     },
-    settingsPath ? [settingsPath] : []
+    [settingsPath, loadedProject?.project?.path].filter(
+      (x: string | undefined) => x !== undefined
+    )
   )
 
   // Add settings commands to the command bar

src/hooks/useFileSystemWatcher.tsx

@@ -12,35 +12,51 @@ type Path = string
 // watcher.addListener(() => { ... }).
 
 export const useFileSystemWatcher = (
-  callback: (path: Path) => Promise<void>,
+  callback: (eventType: string, path: Path) => Promise<void>,
-  dependencyArray: Path[]
+  paths: Path[]
 ): void => {
-  // Track a ref to the callback. This is how we get the callback updated
+  // Used to track this instance of useFileSystemWatcher.
-  // across the NodeJS<->Browser boundary.
+  // Assign to ref so it doesn't change between renders.
-  const callbackRef = useRef<{ fn: (path: Path) => Promise<void> }>({
+  const key = useRef(Math.random().toString())
-    fn: async (_path) => {},
+
-  })
+  const [output, setOutput] = useState<
+    { eventType: string; path: string } | undefined
+  >(undefined)
+
+  // Used to track if paths list changes.
+  const [pathsTracked, setPathsTracked] = useState<Path[]>([])
 
   useEffect(() => {
-    callbackRef.current.fn = callback
+    if (!output) return
-  }, [callback])
+    callback(output.eventType, output.path).catch(reportRejection)
-
+  }, [output])
-  // Used to track if dependencyArrray changes.
-  const [dependencyArrayTracked, setDependencyArrayTracked] = useState<Path[]>(
-    []
-  )
 
   // On component teardown obliterate all watchers.
   useEffect(() => {
     // The hook is useless on web.
     if (!isDesktop()) return
 
+    const cbWatcher = (eventType: string, path: string) => {
+      setOutput({ eventType, path })
+    }
+
+    for (let path of pathsTracked) {
+      // Because functions don't retain refs between NodeJS-Browser I need to
+      // pass an identifying key so we can later remove it.
+      // A way to think of the function call is:
+      // "For this path, add a new handler with this key"
+      // "There can be many keys (functions) per path"
+      // Again if refs were preserved, we wouldn't need to do this. Keys
+      // gives us uniqueness.
+      window.electron.watchFileOn(path, key.current, cbWatcher)
+    }
+
     return () => {
-      for (let path of dependencyArray) {
+      for (let path of pathsTracked) {
-        window.electron.watchFileOff(path)
+        window.electron.watchFileOff(path, key.current)
       }
     }
-  }, [])
+  }, [pathsTracked])
 
   function difference<T>(l1: T[], l2: T[]): [T[], T[]] {
     return [
@@ -49,8 +65,7 @@ export const useFileSystemWatcher = (
     ]
   }
 
-  const hasDiff =
+  const hasDiff = difference(paths, pathsTracked)[0].length !== 0
-    difference(dependencyArray, dependencyArrayTracked)[0].length !== 0
 
   // Removing 1 watcher at a time is only possible because in a filesystem,
   // a path is unique (there can never be two paths with the same name).
@@ -61,19 +76,8 @@ export const useFileSystemWatcher = (
 
     if (!hasDiff) return
 
-    const [pathsRemoved, pathsRemaining] = difference(
+    const [, pathsRemaining] = difference(pathsTracked, paths)
-      dependencyArrayTracked,
+    const [pathsAdded] = difference(paths, pathsTracked)
-      dependencyArray
+    setPathsTracked(pathsRemaining.concat(pathsAdded))
-    )
-    for (let path of pathsRemoved) {
-      window.electron.watchFileOff(path)
-    }
-    const [pathsAdded] = difference(dependencyArray, dependencyArrayTracked)
-    for (let path of pathsAdded) {
-      window.electron.watchFileOn(path, (_eventType: string, path: Path) => {
-        callbackRef.current.fn(path).catch(reportRejection)
-      })
-    }
-    setDependencyArrayTracked(pathsRemaining.concat(pathsAdded))
   }, [hasDiff])
 }

src/lib/codeEditor.ts

@@ -0,0 +1,3 @@
+export const normalizeLineEndings = (str: string, normalized = '\n') => {
+  return str.replace(/\r?\n/g, normalized)
+}

src/lib/routeLoaders.ts

@@ -14,6 +14,7 @@ import { codeManager } from 'lib/singletons'
 import { fileSystemManager } from 'lang/std/fileSystemManager'
 import { getProjectInfo } from './desktop'
 import { createSettings } from './settings/initialSettings'
+import { normalizeLineEndings } from 'lib/codeEditor'
 
 // The root loader simply resolves the settings and any errors that
 // occurred during the settings load
@@ -182,7 +183,3 @@ export const homeLoader: LoaderFunction = async (): Promise<
   }
   return {}
 }
-
-const normalizeLineEndings = (str: string, normalized = '\n') => {
-  return str.replace(/\r?\n/g, normalized)
-}

src/main.ts

@@ -37,8 +37,6 @@ if (!process.env.NODE_ENV)
 // dotenv override when present
 dotenv.config({ path: [`.env.${NODE_ENV}.local`, `.env.${NODE_ENV}`] })
 
-console.log(process.env)
-
 process.env.VITE_KC_API_WS_MODELING_URL ??=
   'wss://api.zoo.dev/ws/modeling/commands'
 process.env.VITE_KC_API_BASE_URL ??= 'https://api.zoo.dev'

src/preload.ts

@@ -30,20 +30,49 @@ const isMac = os.platform() === 'darwin'
 const isWindows = os.platform() === 'win32'
 const isLinux = os.platform() === 'linux'
 
-let fsWatchListeners = new Map<string, ReturnType<typeof chokidar.watch>>()
+let fsWatchListeners = new Map<
+  string,
+  Map<
+    string,
+    {
+      watcher: ReturnType<typeof chokidar.watch>
+      callback: (eventType: string, path: string) => void
+    }
+  >
+>()
 
-const watchFileOn = (path: string, callback: (path: string) => void) => {
+const watchFileOn = (
-  const watcherMaybe = fsWatchListeners.get(path)
+  path: string,
-  if (watcherMaybe) return
+  key: string,
-  const watcher = chokidar.watch(path)
+  callback: (eventType: string, path: string) => void
+) => {
+  let watchers = fsWatchListeners.get(path)
+  if (!watchers) {
+    watchers = new Map()
+  }
+  const watcher = chokidar.watch(path, { depth: 1 })
   watcher.on('all', callback)
-  fsWatchListeners.set(path, watcher)
+  watchers.set(key, { watcher, callback })
+  fsWatchListeners.set(path, watchers)
 }
-const watchFileOff = (path: string) => {
+const watchFileOff = (path: string, key: string) => {
-  const watcher = fsWatchListeners.get(path)
+  const watchers = fsWatchListeners.get(path)
-  if (!watcher) return
+  if (!watchers) return
-  watcher.unwatch(path)
+  const data = watchers.get(key)
-  fsWatchListeners.delete(path)
+  if (!data) {
+    console.warn(
+      "Trying to remove a watcher, callback that doesn't exist anymore. Suspicious."
+    )
+    return
+  }
+  const { watcher, callback } = data
+  watcher.off('all', callback)
+  watchers.delete(key)
+  if (watchers.size === 0) {
+    fsWatchListeners.delete(path)
+  } else {
+    fsWatchListeners.set(path, watchers)
+  }
 }
 const readFile = (path: string) => fs.readFile(path, 'utf-8')
 // It seems like from the node source code this does not actually block but also