ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reload user settings when changed externally (#4097)

Browse Source 
* Reload user settings when changed externally

* Fix to not use any

* Make sure listener doesn't already exist

* Fix up projects reloading

---------

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
This commit is contained in:
49fl
2024-10-07 23:07:18 -04:00
committed by GitHub
parent 7de0b74c16
commit 24cd1b2ea5
11 changed files with 151 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
e2e/playwright/testing-settings.spec.ts
View File

@ -9,6 +9,7 @@ import {
executorInputPath,
} from './test-utils'
import { SaveSettingsPayload, SettingsLevel } from 'lib/settings/settingsTypes'
import { SETTINGS_FILE_NAME } from 'lib/constants'
import {
TEST_SETTINGS_KEY,
TEST_SETTINGS_CORRUPTED,
@ -343,7 +344,7 @@ test.describe('Testing settings', () => {
// Selectors and constants
const errorHeading = page.getByRole('heading', {
name: 'An unextected error occurred',
name: 'An unexpected error occurred',
})
const projectDirLink = page.getByText('Loaded from')
@ -372,7 +373,7 @@ test.describe('Testing settings', () => {
// Selectors and constants
const errorHeading = page.getByRole('heading', {
name: 'An unextected error occurred',
name: 'An unexpected error occurred',
})
const projectDirLink = page.getByText('Loaded from')
@ -384,6 +385,66 @@ test.describe('Testing settings', () => {
}
)
// It was much easier to test the logo color than the background stream color.
test(
'user settings reload on external change, on project and modeling view',
{ tag: '@electron' },
async ({ browserName }, testInfo) => {
const {
electronApp,
page,
dir: projectDirName,
} = await setupElectron({
testInfo,
appSettings: {
app: {
// Doesn't matter what you set it to. It will
// default to 264.5
themeColor: '0',
},
},
})
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()
await expect(logoLink).toHaveCSS('--primary-hue', '264.5')
})
const changeColor = async (color: string) => {
const tempSettingsFilePath = join(projectDirName, SETTINGS_FILE_NAME)
let tomlStr = await fsp.readFile(tempSettingsFilePath, 'utf-8')
tomlStr = tomlStr.replace(/(themeColor = ")[0-9]+(")/, `$1${color}$2`)
await fsp.writeFile(tempSettingsFilePath, tomlStr)
}
await test.step('Check color of logo changed', async () => {
await changeColor('99')
await expect(logoLink).toHaveCSS('--primary-hue', '99')
})
await test.step('Check color of logo changed when in modeling view', async () => {
await page.getByRole('button', { name: 'New project' }).click()
await page.getByTestId('project-link').first().click()
await page.getByRole('button', { name: 'Dismiss' }).click()
await changeColor('58')
await expect(logoLink).toHaveCSS('--primary-hue', '58')
})
await test.step('Check going back to projects view still changes the color', async () => {
await logoLink.click()
await expect(projectDirLink).toBeVisible()
await changeColor('21')
await expect(logoLink).toHaveCSS('--primary-hue', '21')
})
await electronApp.close()
}
)
test(
`Closing settings modal should go back to the original file being viewed`,
{ tag: '@electron' },

1
interface.d.ts vendored
View File

@ -23,7 +23,6 @@ export interface IElectronAPI {
callback: (eventType: string, path: string) => void
) => void
watchFileOff: (path: string) => void
watchFileObliterate: () => void
readFile: (path: string) => ReturnType<fs.readFile>
writeFile: (
path: string,

1
package.json
View File

@ -36,6 +36,7 @@
"@xstate/inspect": "^0.8.0",
"@xstate/react": "^4.1.1",
"bonjour-service": "^1.2.1",
"chokidar": "^4.0.1",
"codemirror": "^6.0.1",
"decamelize": "^6.0.0",
"electron-squirrel-startup": "^1.0.1",

38
src/components/SettingsAuthProvider.tsx
View File

@ -1,9 +1,10 @@
import { trap } from 'lib/trap'
import { useMachine } from '@xstate/react'
import { useNavigate, useRouteLoaderData, useLocation } from 'react-router-dom'
import { PATHS } from 'lib/paths'
import { authMachine, TOKEN_PERSIST_KEY } from '../machines/authMachine'
import withBaseUrl from '../lib/withBaseURL'
import React, { createContext, useEffect } from 'react'
import React, { createContext, useEffect, useState } from 'react'
import useStateMachineCommands from '../hooks/useStateMachineCommands'
import { settingsMachine } from 'machines/settingsMachine'
import { toast } from 'react-hot-toast'
@ -15,7 +16,6 @@ import {
} from 'lib/theme'
import decamelize from 'decamelize'
import { Actor, AnyStateMachine, ContextFrom, Prop, StateFrom } from 'xstate'
import { isDesktop } from 'lib/isDesktop'
import { authCommandBarConfig } from 'lib/commandBarConfigs/authCommandConfig'
import {
kclManager,
@ -33,8 +33,14 @@ import {
import { useCommandsContext } from 'hooks/useCommandsContext'
import { Command } from 'lib/commandTypes'
import { BaseUnit } from 'lib/settings/settingsTypes'
import { saveSettings } from 'lib/settings/settingsUtils'
import {
saveSettings,
loadAndValidateSettings,
} from 'lib/settings/settingsUtils'
import { reportRejection } from 'lib/trap'
import { getAppSettingsFilePath } from 'lib/desktop'
import { isDesktop } from 'lib/isDesktop'
import { useFileSystemWatcher } from 'hooks/useFileSystemWatcher'
type MachineContext<T extends AnyStateMachine> = {
state: StateFrom<T>
@ -99,6 +105,9 @@ export const SettingsAuthProviderBase = ({
const location = useLocation()
const navigate = useNavigate()
const { commandBarSend } = useCommandsContext()
const [settingsPath, setSettingsPath] = useState<string | undefined>(
undefined
)
const [settingsState, settingsSend, settingsActor] = useMachine(
settingsMachine.provide({
@ -191,7 +200,11 @@ export const SettingsAuthProviderBase = ({
console.error('Error executing AST after settings change', e)
}
},
persistSettings: ({ context }) => {
persistSettings: ({ context, event }) => {
// Without this, when a user changes the file, it'd
// create a detection loop with the file-system watcher.
if (event.doNotPersist) return
// eslint-disable-next-line @typescript-eslint/no-floating-promises
saveSettings(context, loadedProject?.project?.path)
},
@ -201,6 +214,23 @@ export const SettingsAuthProviderBase = ({
)
settingsStateRef = settingsState.context
useEffect(() => {
if (!isDesktop()) return
getAppSettingsFilePath().then(setSettingsPath).catch(trap)
}, [])
useFileSystemWatcher(
async () => {
const data = await loadAndValidateSettings(loadedProject?.project?.path)
settingsSend({
type: 'Set all settings',
settings: data.settings,
doNotPersist: true,
})
},
settingsPath ? [settingsPath] : []
)
// Add settings commands to the command bar
// They're treated slightly differently than other commands
// Because their state machine doesn't have a meaningful .nextEvents,

24
src/hooks/useFileSystemWatcher.tsx
View File

@ -1,4 +1,5 @@
import { isDesktop } from 'lib/isDesktop'
import { reportRejection } from 'lib/trap'
import { useEffect, useState, useRef } from 'react'
type Path = string
@ -11,13 +12,13 @@ type Path = string
// watcher.addListener(() => { ... }).
export const useFileSystemWatcher = (
callback: (path: Path) => void,
callback: (path: Path) => Promise<void>,
dependencyArray: 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) => void }>({
fn: (_path) => {},
const callbackRef = useRef<{ fn: (path: Path) => Promise<void> }>({
fn: async (_path) => {},
})
useEffect(() => {
@ -35,7 +36,9 @@ export const useFileSystemWatcher = (
if (!isDesktop()) return
return () => {
window.electron.watchFileObliterate()
for (let path of dependencyArray) {
window.electron.watchFileOff(path)
}
}
}, [])
@ -46,6 +49,9 @@ export const useFileSystemWatcher = (
]
}
const hasDiff =
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).
// Otherwise we would have to obliterate() the whole list and reconstruct it.
@ -53,6 +59,8 @@ export const useFileSystemWatcher = (
// The hook is useless on web.
if (!isDesktop()) return
if (!hasDiff) return
const [pathsRemoved, pathsRemaining] = difference(
dependencyArrayTracked,
dependencyArray
@ -62,10 +70,10 @@ export const useFileSystemWatcher = (
}
const [pathsAdded] = difference(dependencyArray, dependencyArrayTracked)
for (let path of pathsAdded) {
window.electron.watchFileOn(path, (_eventType: string, path: Path) =>
callbackRef.current.fn(path)
)
window.electron.watchFileOn(path, (_eventType: string, path: Path) => {
callbackRef.current.fn(path).catch(reportRejection)
})
}
setDependencyArrayTracked(pathsRemaining.concat(pathsAdded))
}, [difference(dependencyArray, dependencyArrayTracked)[0].length !== 0])
}, [hasDiff])
}

2
src/lib/desktop.ts
View File

@ -379,7 +379,7 @@ const getAppFolderName = () => {
return window.electron.packageJson.name
}
const getAppSettingsFilePath = async () => {
export const getAppSettingsFilePath = async () => {
const isTestEnv = window.electron.process.env.IS_PLAYWRIGHT === 'true'
const testSettingsPath = window.electron.process.env.TEST_SETTINGS_FILE_KEY
const appConfig = await window.electron.getPath('appData')

12
src/lib/settings/settingsUtils.ts
View File

@ -177,14 +177,14 @@ export async function loadAndValidateSettings(
if (err(appSettingsPayload)) return Promise.reject(appSettingsPayload)
const settings = createSettings()
let settingsNext = createSettings()
// Because getting the default directory is async, we need to set it after
if (onDesktop) {
settings.app.projectDirectory.default = await getInitialDefaultDir()
}
setSettingsAtLevel(
settings,
settingsNext = setSettingsAtLevel(
settingsNext,
'user',
configurationToSettingsPayload(appSettingsPayload)
)
@ -199,8 +199,8 @@ export async function loadAndValidateSettings(
return Promise.reject(new Error('Invalid project settings'))
const projectSettingsPayload = projectSettings
setSettingsAtLevel(
settings,
settingsNext = setSettingsAtLevel(
settingsNext,
'project',
projectConfigurationToSettingsPayload(projectSettingsPayload)
)
@ -208,7 +208,7 @@ export async function loadAndValidateSettings(
// Return the settings object
return {
settings,
settings: settingsNext,
configuration: appSettingsPayload,
}
}

5
src/machines/settingsMachine.ts
View File

@ -19,7 +19,7 @@ export const settingsMachine = setup({
types: {
context: {} as ReturnType<typeof createSettings>,
input: {} as ReturnType<typeof createSettings>,
events: {} as
events: {} as (
| WildcardSetEvent<SettingsPaths>
| SetEventTypes
| {
@ -34,7 +34,8 @@ export const settingsMachine = setup({
type: 'Reset settings'
level: SettingsLevel
}
| { type: 'Set all settings'; settings: typeof settings },
| { type: 'Set all settings'; settings: typeof settings }
) & { doNotPersist?: boolean },
},
actions: {
setEngineTheme: () => {},

37
src/preload.ts
View File

@ -5,6 +5,7 @@ import os from 'node:os'
import fsSync from 'node:fs'
import packageJson from '../package.json'
import { MachinesListing } from 'lib/machineManager'
import chokidar from 'chokidar'
const open = (args: any) => ipcRenderer.invoke('dialog.showOpenDialog', args)
const save = (args: any) => ipcRenderer.invoke('dialog.showSaveDialog', args)
@ -23,36 +24,21 @@ const isMac = os.platform() === 'darwin'
const isWindows = os.platform() === 'win32'
const isLinux = os.platform() === 'linux'
let fsWatchListeners = new Map<
string,
{
watcher: fsSync.FSWatcher
callback: (eventType: string, path: string) => void
}
>()
let fsWatchListeners = new Map<string, ReturnType<typeof chokidar.watch>>()
const watchFileOn = (
path: string,
callback: (eventType: string, path: string) => void
) => {
const watcher = fsSync.watch(path)
watcher.on('change', callback)
fsWatchListeners.set(path, { watcher, callback })
const watchFileOn = (path: string, callback: (path: string) => void) => {
const watcherMaybe = fsWatchListeners.get(path)
if (watcherMaybe) return
const watcher = chokidar.watch(path)
watcher.on('all', callback)
fsWatchListeners.set(path, watcher)
}
const watchFileOff = (path: string) => {
const entry = fsWatchListeners.get(path)
if (!entry) return
const { watcher, callback } = entry
watcher.off('change', callback)
watcher.close()
const watcher = fsWatchListeners.get(path)
if (!watcher) return
watcher.unwatch(path)
fsWatchListeners.delete(path)
}
const watchFileObliterate = () => {
for (let [pathAsKey] of fsWatchListeners) {
watchFileOff(pathAsKey)
}
fsWatchListeners = new Map()
}
const readFile = (path: string) => fs.readFile(path, 'utf-8')
// It seems like from the node source code this does not actually block but also
// don't trust me on that (jess).
@ -103,7 +89,6 @@ contextBridge.exposeInMainWorld('electron', {
// exported.
watchFileOn,
watchFileOff,
watchFileObliterate,
readFile,
writeFile,
exists,

2
src/routes/Home.tsx
View File

@ -176,7 +176,7 @@ const Home = () => {
// Re-read projects listing if the projectDir has any updates.
useFileSystemWatcher(
() => {
async () => {
setProjectsLoaderTrigger(projectsLoaderTrigger + 1)
},
projectsDir ? [projectsDir] : []

43
yarn.lock
View File

@ -3780,6 +3780,13 @@ chokidar@^3.5.3:
optionalDependencies:
fsevents "~2.3.2"
chokidar@^4.0.1:
version "4.0.1"
resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-4.0.1.tgz#4a6dff66798fb0f72a94f616abbd7e1a19f31d41"
integrity sha512-n8enUVCED/KVRQlab1hr3MVpcVMvxtZjmEa956u+4YijlmQED223XMSYj2tLuKvr4jcCTzNNMpQDUer72MMmzA==
dependencies:
readdirp "^4.0.1"
chownr@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/chownr/-/chownr-2.0.0.tgz#15bfbe53d2eab4cf70f18a8cd68ebe5b3cb1dece"
@ -8155,6 +8162,11 @@ readable-stream@~2.3.6:
string_decoder "~1.1.1"
util-deprecate "~1.0.1"
readdirp@^4.0.1:
version "4.0.2"
resolved "https://registry.yarnpkg.com/readdirp/-/readdirp-4.0.2.tgz#388fccb8b75665da3abffe2d8f8ed59fe74c230a"
integrity sha512-yDMz9g+VaZkqBYS/ozoBJwaBhTbZo3UNYQHNRw1D3UFQB8oHB4uS/tAODO+ZLjGWmUbKnIlOWO+aaIiAxrUWHA==
readdirp@~3.6.0:
version "3.6.0"
resolved "https://registry.yarnpkg.com/readdirp/-/readdirp-3.6.0.tgz#74a370bd857116e245b29cc97340cd431a02a6c7"
@ -8773,16 +8785,7 @@ string-natural-compare@^3.0.1:
resolved "https://registry.yarnpkg.com/string-natural-compare/-/string-natural-compare-3.0.1.tgz#7a42d58474454963759e8e8b7ae63d71c1e7fdf4"
integrity sha512-n3sPwynL1nwKi3WJ6AIsClwBMa0zTi54fn2oLU6ndfTSIO05xaznjSf15PcBZU6FNWbmN5Q6cxT4V5hGvB4taw==
"string-width-cjs@npm:string-width@^4.2.0":
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"
"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@ -8876,14 +8879,7 @@ string_decoder@~1.1.1:
dependencies:
safe-buffer "~5.1.0"
"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"
strip-ansi@^6.0.0, strip-ansi@^6.0.1:
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@ -9757,16 +9753,7 @@ word-wrap@^1.2.3, word-wrap@^1.2.5:
resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.5.tgz#d2c45c6dd4fbce621a66f136cbe328afd0410b34"
integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"
wrap-ansi@^7.0.0:
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==