diff --git a/e2e/playwright/code-pane-and-errors.spec.ts b/e2e/playwright/code-pane-and-errors.spec.ts index 0de251622..8bfe6d194 100644 --- a/e2e/playwright/code-pane-and-errors.spec.ts +++ b/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() + } +) diff --git a/e2e/playwright/file-tree.spec.ts b/e2e/playwright/file-tree.spec.ts index 052a5c25a..fc5a2f35e 100644 --- a/e2e/playwright/file-tree.spec.ts +++ b/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/ 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/ 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() + } + ) }) diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 5193f8726..0f503dbc3 100644 --- a/e2e/playwright/test-utils.ts +++ b/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') diff --git a/e2e/playwright/testing-settings.spec.ts b/e2e/playwright/testing-settings.spec.ts index 63aa05f46..9a7ff7516 100644 --- a/e2e/playwright/testing-settings.spec.ts +++ b/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' }, diff --git a/interface.d.ts b/interface.d.ts index 77d5e07b7..e0f07c376 100644 --- a/interface.d.ts +++ b/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 writeFile: ( path: string, diff --git a/src/components/FileTree.tsx b/src/components/FileTree.tsx index 44d098539..c24f9a60a 100644 --- a/src/components/FileTree.tsx +++ b/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) { 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) => - fileSend({ - type: 'Set selected directory', - directory: fileOrDir, - }) - } - onFocusCapture={(e) => - fileSend({ - type: 'Set selected directory', - directory: fileOrDir, - }) - } + onClickCapture={clickDirectory} + onFocusCapture={clickDirectory} 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 - useEffect(() => { - fileSend({ type: 'Refresh' }) - }, [documentHasFocus]) + // Refresh the file tree when there are changes. + useFileSystemWatcher( + async (eventType, path) => { + 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 (
-
    { - fileSend({ - type: 'Set selected directory', - directory: fileContext.project, - }) - }} - > +
      {sortProject(fileContext.project?.children || []).map((fileOrDir) => ( { + // 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 diff --git a/src/hooks/useFileSystemWatcher.tsx b/src/hooks/useFileSystemWatcher.tsx index 779e58dff..210b31e11 100644 --- a/src/hooks/useFileSystemWatcher.tsx +++ b/src/hooks/useFileSystemWatcher.tsx @@ -12,35 +12,51 @@ type Path = string // watcher.addListener(() => { ... }). export const useFileSystemWatcher = ( - callback: (path: Path) => Promise, - dependencyArray: Path[] + callback: (eventType: string, path: Path) => Promise, + paths: Path[] ): void => { - // Track a ref to the callback. This is how we get the callback updated - // across the NodeJS<->Browser boundary. - const callbackRef = useRef<{ fn: (path: Path) => Promise }>({ - fn: async (_path) => {}, - }) + // Used to track this instance of useFileSystemWatcher. + // Assign to ref so it doesn't change between renders. + const key = useRef(Math.random().toString()) + + const [output, setOutput] = useState< + { eventType: string; path: string } | undefined + >(undefined) + + // Used to track if paths list changes. + const [pathsTracked, setPathsTracked] = useState([]) useEffect(() => { - callbackRef.current.fn = callback - }, [callback]) - - // Used to track if dependencyArrray changes. - const [dependencyArrayTracked, setDependencyArrayTracked] = useState( - [] - ) + if (!output) return + callback(output.eventType, output.path).catch(reportRejection) + }, [output]) // 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) { - window.electron.watchFileOff(path) + for (let path of pathsTracked) { + window.electron.watchFileOff(path, key.current) } } - }, []) + }, [pathsTracked]) function difference(l1: T[], l2: T[]): [T[], T[]] { return [ @@ -49,8 +65,7 @@ export const useFileSystemWatcher = ( ] } - const hasDiff = - difference(dependencyArray, dependencyArrayTracked)[0].length !== 0 + const hasDiff = difference(paths, pathsTracked)[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( - dependencyArrayTracked, - dependencyArray - ) - 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)) + const [, pathsRemaining] = difference(pathsTracked, paths) + const [pathsAdded] = difference(paths, pathsTracked) + setPathsTracked(pathsRemaining.concat(pathsAdded)) }, [hasDiff]) } diff --git a/src/lib/codeEditor.ts b/src/lib/codeEditor.ts new file mode 100644 index 000000000..ca853a044 --- /dev/null +++ b/src/lib/codeEditor.ts @@ -0,0 +1,3 @@ +export const normalizeLineEndings = (str: string, normalized = '\n') => { + return str.replace(/\r?\n/g, normalized) +} diff --git a/src/lib/routeLoaders.ts b/src/lib/routeLoaders.ts index 719746367..45f1d9c84 100644 --- a/src/lib/routeLoaders.ts +++ b/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) -} diff --git a/src/main.ts b/src/main.ts index 9abe516d2..93400b1ce 100644 --- a/src/main.ts +++ b/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' diff --git a/src/preload.ts b/src/preload.ts index 841eb9976..d032e327f 100644 --- a/src/preload.ts +++ b/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>() +let fsWatchListeners = new Map< + string, + Map< + string, + { + watcher: ReturnType + callback: (eventType: string, path: string) => void + } + > +>() -const watchFileOn = (path: string, callback: (path: string) => void) => { - const watcherMaybe = fsWatchListeners.get(path) - if (watcherMaybe) return - const watcher = chokidar.watch(path) +const watchFileOn = ( + path: string, + key: string, + 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 watcher = fsWatchListeners.get(path) - if (!watcher) return - watcher.unwatch(path) - fsWatchListeners.delete(path) +const watchFileOff = (path: string, key: string) => { + const watchers = fsWatchListeners.get(path) + if (!watchers) return + const data = watchers.get(key) + 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