From 941eacd559aeea5eebbab6a9e44e87946e734df4 Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Tue, 6 May 2025 11:48:45 -0500 Subject: [PATCH] [BUG]: `split('/')` caused bug on windows (#6697) * fix: trying to figure out this pathing issue * fix: found the bug * fix: adding linter rule * fix: rule for join('/') as well * fix: removing useless string template * fix: removing useless string template * fix: ???? What ???? * fix: remove unused import * fix: circular dep was added when I cleaned up the path logic, fixed the circular dep by passing args from the parent function --- .eslintrc.json | 8 ++++ src/editor/plugins/lsp/copilot/index.ts | 1 + src/hooks/useEngineConnectionSubscriptions.ts | 3 +- src/lang/wasmUtils.ts | 3 +- .../applicationCommandConfig.ts | 10 ++-- .../projectsCommandConfig.ts | 2 +- src/lib/desktop.test.ts | 9 ++-- src/lib/paths.ts | 46 ++++++++++++++++--- src/lib/routeLoaders.ts | 8 +++- src/lib/textToCad.ts | 2 +- src/machines/systemIO/systemIOMachine.ts | 23 ++++++++-- .../systemIO/systemIOMachineDesktop.ts | 8 ++-- src/machines/systemIO/systemIOMachineWeb.ts | 4 +- src/routes/Onboarding/utils.tsx | 2 +- 14 files changed, 100 insertions(+), 29 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index a7830ff36..3ac660aab 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -87,6 +87,14 @@ { "selector": "CallExpression[callee.object.name='TOML'][callee.property.name='parse']", "message": "Do not use TOML.parse directly. Use the wrappers in test-utils instead like tomlToSettings." + }, + { + "selector": "CallExpression[callee.property.name='split'] > Literal[value='/']", + "message": "Avoid using split with '/'." + }, + { + "selector": "CallExpression[callee.property.name='join'] > Literal[value='/']", + "message": "Avoid using join with '/'." } ], "no-restricted-imports": [ diff --git a/src/editor/plugins/lsp/copilot/index.ts b/src/editor/plugins/lsp/copilot/index.ts index 729531e06..4d31fb2bf 100644 --- a/src/editor/plugins/lsp/copilot/index.ts +++ b/src/editor/plugins/lsp/copilot/index.ts @@ -323,6 +323,7 @@ export class CompletionRequester implements PluginValue { tabSize: state.facet(EditorState.tabSize), indentSize: 1, insertSpaces: true, + // eslint-disable-next-line path: dUri.split('/').pop()!, uri: dUri, relativePath: dUri.replace('file://', ''), diff --git a/src/hooks/useEngineConnectionSubscriptions.ts b/src/hooks/useEngineConnectionSubscriptions.ts index e5411d39e..a14ec7d10 100644 --- a/src/hooks/useEngineConnectionSubscriptions.ts +++ b/src/hooks/useEngineConnectionSubscriptions.ts @@ -34,6 +34,7 @@ import type { ExtrudeFacePlane, } from '@src/machines/modelingMachine' import toast from 'react-hot-toast' +import { localModuleSafePathSplit } from '@src/lib/paths' export function useEngineConnectionSubscriptions() { const { send, context, state } = useModelingContext() @@ -208,7 +209,7 @@ export function useEngineConnectionSubscriptions() { return } if (importDetails?.type === 'Local') { - const paths = importDetails.value.split('/') + const paths = localModuleSafePathSplit(importDetails.value) const fileName = paths[paths.length - 1] showSketchOnImportToast(fileName) } else if ( diff --git a/src/lang/wasmUtils.ts b/src/lang/wasmUtils.ts index 0fd25243d..373b66e14 100644 --- a/src/lang/wasmUtils.ts +++ b/src/lang/wasmUtils.ts @@ -2,6 +2,7 @@ import { import_file_extensions, relevant_file_extensions, } from '@rust/kcl-wasm-lib/pkg/kcl_wasm_lib' +import { webSafeJoin, webSafePathSplit } from '@src/lib/paths' import { init, reloadModule } from '@src/lib/wasm_lib_wrapper' export const wasmUrl = () => { @@ -12,7 +13,7 @@ export const wasmUrl = () => { const fullUrl = document.location.protocol.includes('http') ? document.location.origin + '/kcl_wasm_lib_bg.wasm' : document.location.protocol + - document.location.pathname.split('/').slice(0, -1).join('/') + + webSafeJoin(webSafePathSplit(document.location.pathname).slice(0, -1)) + '/kcl_wasm_lib_bg.wasm' return fullUrl diff --git a/src/lib/commandBarConfigs/applicationCommandConfig.ts b/src/lib/commandBarConfigs/applicationCommandConfig.ts index e666dc23d..ddc9ae737 100644 --- a/src/lib/commandBarConfigs/applicationCommandConfig.ts +++ b/src/lib/commandBarConfigs/applicationCommandConfig.ts @@ -13,6 +13,7 @@ import { import toast from 'react-hot-toast' import { reportRejection } from '@src/lib/trap' import { relevantFileExtensions } from '@src/lang/wasmUtils' +import { getStringAfterLastSeparator, webSafePathSplit } from '@src/lib/paths' export function createApplicationCommands({ systemIOActor, @@ -115,7 +116,8 @@ export function createApplicationCommands({ : requestedProjectName if (data.source === 'kcl-samples' && data.sample) { - const pathParts = data.sample.split('/') + // This is web safe because the values are taken from manifest.json not from the disk when selecting + const pathParts = webSafePathSplit(data.sample) const projectPathPart = pathParts[0] const primaryKclFile = pathParts[1] const folderNameBecomesKCLFileName = projectPathPart + FILE_EXT @@ -140,7 +142,7 @@ export function createApplicationCommands({ type: SystemIOMachineEvents.importFileFromURL, data: { requestedProjectName: uniqueNameIfNeeded, - requestedFileName: folderNameBecomesKCLFileName, + requestedFileNameWithExtension: folderNameBecomesKCLFileName, requestedCode: code, }, }) @@ -148,14 +150,14 @@ export function createApplicationCommands({ .catch(reportError) } else if (data.source === 'local' && data.path) { const clonePath = data.path - const fileWithExtension = clonePath.split('/').pop() + const fileNameWithExtension = getStringAfterLastSeparator(clonePath) const readFileContentsAndCreateNewFile = async () => { const text = await window.electron.readFile(clonePath, 'utf8') systemIOActor.send({ type: SystemIOMachineEvents.importFileFromURL, data: { requestedProjectName: uniqueNameIfNeeded, - requestedFileName: fileWithExtension, + requestedFileNameWithExtension: fileNameWithExtension, requestedCode: text, }, }) diff --git a/src/lib/commandBarConfigs/projectsCommandConfig.ts b/src/lib/commandBarConfigs/projectsCommandConfig.ts index d44703153..7e6a7c87c 100644 --- a/src/lib/commandBarConfigs/projectsCommandConfig.ts +++ b/src/lib/commandBarConfigs/projectsCommandConfig.ts @@ -190,7 +190,7 @@ export function createProjectCommands({ data: { requestedProjectName: record.projectName, requestedCode: record.code, - requestedFileName: record.name, + requestedFileNameWithExtension: record.name, }, }) } diff --git a/src/lib/desktop.test.ts b/src/lib/desktop.test.ts index 2fb71a935..f30571c8d 100644 --- a/src/lib/desktop.test.ts +++ b/src/lib/desktop.test.ts @@ -5,6 +5,7 @@ import type { Configuration } from '@rust/kcl-lib/bindings/Configuration' import { initPromise } from '@src/lang/wasmUtils' import { listProjects } from '@src/lib/desktop' import type { DeepPartial } from '@src/lib/types' +import { webSafeJoin, webSafePathSplit } from '@src/lib/paths' beforeAll(async () => { await initPromise @@ -76,13 +77,15 @@ describe('desktop utilities', () => { // Setup default mock implementations mockElectron.path.join.mockImplementation((...parts: string[]) => - parts.join('/') + webSafeJoin(parts) ) mockElectron.path.basename.mockImplementation((path: string) => - path.split('/').pop() + // The tests is hard coded to / so webSafe is defaulted to / + webSafePathSplit(path).pop() ) mockElectron.path.dirname.mockImplementation((path: string) => - path.split('/').slice(0, -1).join('/') + // The tests is hard coded to / so webSafe is defaulted to / + webSafeJoin(webSafePathSplit(path).slice(0, -1)) ) // Mock readdir to return the entries for the given path diff --git a/src/lib/paths.ts b/src/lib/paths.ts index 6396fb445..fe605215e 100644 --- a/src/lib/paths.ts +++ b/src/lib/paths.ts @@ -7,9 +7,7 @@ import { BROWSER_PROJECT_NAME, FILE_EXT, } from '@src/lib/constants' -import { readAppSettingsFile } from '@src/lib/desktop' import { isDesktop } from '@src/lib/isDesktop' -import { readLocalStorageAppSettingsFile } from '@src/lib/settings/settingsUtils' import { err } from '@src/lib/trap' import type { DeepPartial } from '@src/lib/types' import { ONBOARDING_SUBPATHS } from '@src/lib/onboardingPaths' @@ -54,6 +52,8 @@ export const PATHS = { export const BROWSER_PATH = `%2F${BROWSER_PROJECT_NAME}%2F${BROWSER_FILE_NAME}${FILE_EXT}` export async function getProjectMetaByRouteId( + readAppSettingsFile: () => Promise>, + readLocalStorageAppSettingsFile: () => DeepPartial | Error, id?: string, configuration?: DeepPartial | Error ): Promise { @@ -145,10 +145,12 @@ export function parseProjectRoute( * /dog/cat */ export function joinRouterPaths(...parts: string[]): string { - return `/${parts - .map((part) => part.replace(/^\/+|\/+$/g, '')) // Remove leading/trailing slashes - .filter((part) => part.length > 0) // Remove empty segments - .join('/')}` + const cleanedUpPath = webSafeJoin( + parts + .map((part) => part.replace(/^\/+|\/+$/g, '')) // Remove leading/trailing slashes + .filter((part) => part.length > 0) + ) // Remove empty segments + return `/${cleanedUpPath}` } /** @@ -186,3 +188,35 @@ export function safeEncodeForRouterPaths(dynamicValue: string): string { export function getStringAfterLastSeparator(path: string): string { return path.split(window.electron.sep).pop() || '' } + +/** + * Use this for only web related paths not paths in OS or on disk + * e.g. document.location.pathname + */ +export function webSafePathSplit(path: string): string[] { + const webSafeSep = '/' + return path.split(webSafeSep) +} + +export function webSafeJoin(paths: string[]): string { + const webSafeSep = '/' + return paths.join(webSafeSep) +} + +/** + * Splits any paths safely based on the runtime + */ +export function desktopSafePathSplit(path: string): string[] { + return isDesktop() + ? path.split(window?.electron?.sep) + : webSafePathSplit(path) +} + +export function desktopSafePathJoin(paths: string[]): string { + return isDesktop() ? paths.join(window?.electron?.sep) : webSafeJoin(paths) +} + +export function localModuleSafePathSplit(path: string) { + const modulePathSafeSep = '/' + return path.split(modulePathSafeSep) +} diff --git a/src/lib/routeLoaders.ts b/src/lib/routeLoaders.ts index 020efedfb..29bac8e75 100644 --- a/src/lib/routeLoaders.ts +++ b/src/lib/routeLoaders.ts @@ -13,7 +13,10 @@ import { import { getProjectInfo } from '@src/lib/desktop' import { isDesktop } from '@src/lib/isDesktop' import { BROWSER_PATH, PATHS, getProjectMetaByRouteId } from '@src/lib/paths' -import { loadAndValidateSettings } from '@src/lib/settings/settingsUtils' +import { + loadAndValidateSettings, + readLocalStorageAppSettingsFile, +} from '@src/lib/settings/settingsUtils' import { codeManager } from '@src/lib/singletons' import type { FileLoaderData, @@ -21,6 +24,7 @@ import type { IndexLoaderData, } from '@src/lib/types' import { settingsActor } from '@src/lib/singletons' +import { readAppSettingsFile } from '@src/lib/desktop' export const fileLoader: LoaderFunction = async ( routerData @@ -29,6 +33,8 @@ export const fileLoader: LoaderFunction = async ( let { configuration } = await loadAndValidateSettings() const projectPathData = await getProjectMetaByRouteId( + readAppSettingsFile, + readLocalStorageAppSettingsFile, params.id, configuration ) diff --git a/src/lib/textToCad.ts b/src/lib/textToCad.ts index d65f433d8..3764935c3 100644 --- a/src/lib/textToCad.ts +++ b/src/lib/textToCad.ts @@ -213,7 +213,7 @@ export async function submitAndAwaitTextToKclSystemIO({ data: { requestedProjectName: projectName, requestedCode: value.code, - requestedFileName: newFileName, + requestedFileNameWithExtension: newFileName, }, }) } diff --git a/src/machines/systemIO/systemIOMachine.ts b/src/machines/systemIO/systemIOMachine.ts index 93984de03..411c79f78 100644 --- a/src/machines/systemIO/systemIOMachine.ts +++ b/src/machines/systemIO/systemIOMachine.ts @@ -12,6 +12,17 @@ import toast from 'react-hot-toast' import { assertEvent, assign, fromPromise, setup } from 'xstate' import type { AppMachineContext } from '@src/lib/types' +/** + * /some/dir = directoryPath + * report = fileNameWithoutExtension + * report.csv = fileNameWithExtension + * /some/dir/report.csv = absolutePathToFileNameWithExtension + * /some/dir/report = aboslutePathTOFileNameWithoutExtension + * /some/dir/dreport = absolutePathToDirectory + * some/dir/report = relativePathToDirectory + * some/dir/report = relativePathFileWithoutExtension + */ + /** * Handles any system level I/O for folders and files * This machine will be initializes once within the applications runtime @@ -69,7 +80,7 @@ export const systemIOMachine = setup({ type: SystemIOMachineEvents.createKCLFile data: { requestedProjectName: string - requestedFileName: string + requestedFileNameWithExtension: string requestedCode: string } } @@ -77,7 +88,7 @@ export const systemIOMachine = setup({ type: SystemIOMachineEvents.importFileFromURL data: { requestedProjectName: string - requestedFileName: string + requestedFileNameWithExtension: string requestedCode: string requestedSubRoute?: string } @@ -229,7 +240,7 @@ export const systemIOMachine = setup({ input: { context: SystemIOContext requestedProjectName: string - requestedFileName: string + requestedFileNameWithExtension: string requestedCode: string rootContext: AppMachineContext requestedSubRoute?: string @@ -444,7 +455,8 @@ export const systemIOMachine = setup({ return { context, requestedProjectName: event.data.requestedProjectName, - requestedFileName: event.data.requestedFileName, + requestedFileNameWithExtension: + event.data.requestedFileNameWithExtension, requestedCode: event.data.requestedCode, rootContext: self.system.get('root').getSnapshot().context, } @@ -467,7 +479,8 @@ export const systemIOMachine = setup({ return { context, requestedProjectName: event.data.requestedProjectName, - requestedFileName: event.data.requestedFileName, + requestedFileNameWithExtension: + event.data.requestedFileNameWithExtension, requestedSubRoute: event.data.requestedSubRoute, requestedCode: event.data.requestedCode, rootContext: self.system.get('root').getSnapshot().context, diff --git a/src/machines/systemIO/systemIOMachineDesktop.ts b/src/machines/systemIO/systemIOMachineDesktop.ts index 4a382477b..afd13c2c6 100644 --- a/src/machines/systemIO/systemIOMachineDesktop.ts +++ b/src/machines/systemIO/systemIOMachineDesktop.ts @@ -155,14 +155,15 @@ export const systemIOMachineDesktop = systemIOMachine.provide({ input: { context: SystemIOContext requestedProjectName: string - requestedFileName: string + requestedFileNameWithExtension: string requestedCode: string rootContext: AppMachineContext requestedSubRoute?: string } }) => { const requestedProjectName = input.requestedProjectName - const requestedFileName = input.requestedFileName + const requestedFileNameWithExtension = + input.requestedFileNameWithExtension const requestedCode = input.requestedCode const folders = input.context.folders @@ -190,9 +191,10 @@ export const systemIOMachineDesktop = systemIOMachine.provide({ newProjectName ) const { name: newFileName } = getNextFileName({ - entryName: requestedFileName, + entryName: requestedFileNameWithExtension, baseDir, }) + const configuration = await readAppSettingsFile() // Create the project around the file if newProject diff --git a/src/machines/systemIO/systemIOMachineWeb.ts b/src/machines/systemIO/systemIOMachineWeb.ts index 67034695e..3d17c5ada 100644 --- a/src/machines/systemIO/systemIOMachineWeb.ts +++ b/src/machines/systemIO/systemIOMachineWeb.ts @@ -17,7 +17,7 @@ export const systemIOMachineWeb = systemIOMachine.provide({ input: { context: SystemIOContext requestedProjectName: string - requestedFileName: string + requestedFileNameWithExtension: string requestedCode: string rootContext: AppMachineContext requestedSubRoute?: string @@ -42,7 +42,7 @@ export const systemIOMachineWeb = systemIOMachine.provide({ await input.rootContext.kclManager.executeCode() return { message: 'File overwritten successfully', - fileName: input.requestedFileName, + fileName: input.requestedFileNameWithExtension, projectName: '', subRoute: input.requestedSubRoute || '', } diff --git a/src/routes/Onboarding/utils.tsx b/src/routes/Onboarding/utils.tsx index 87361049c..8430e9d3a 100644 --- a/src/routes/Onboarding/utils.tsx +++ b/src/routes/Onboarding/utils.tsx @@ -253,7 +253,7 @@ export async function acceptOnboarding(deps: OnboardingUtilDeps) { type: SystemIOMachineEvents.importFileFromURL, data: { requestedProjectName: ONBOARDING_PROJECT_NAME, - requestedFileName: DEFAULT_PROJECT_KCL_FILE, + requestedFileNameWithExtension: DEFAULT_PROJECT_KCL_FILE, requestedCode: bracket, requestedSubRoute: joinRouterPaths( PATHS.ONBOARDING.INDEX,