[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
This commit is contained in:
Kevin Nadro
2025-05-06 11:48:45 -05:00
committed by GitHub
parent e06a09ed42
commit 941eacd559
14 changed files with 100 additions and 29 deletions

View File

@ -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": [

View File

@ -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://', ''),

View File

@ -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 (

View File

@ -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

View File

@ -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,
},
})

View File

@ -190,7 +190,7 @@ export function createProjectCommands({
data: {
requestedProjectName: record.projectName,
requestedCode: record.code,
requestedFileName: record.name,
requestedFileNameWithExtension: record.name,
},
})
}

View File

@ -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

View File

@ -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<DeepPartial<Configuration>>,
readLocalStorageAppSettingsFile: () => DeepPartial<Configuration> | Error,
id?: string,
configuration?: DeepPartial<Configuration> | Error
): Promise<ProjectRoute | undefined> {
@ -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)
}

View File

@ -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
)

View File

@ -213,7 +213,7 @@ export async function submitAndAwaitTextToKclSystemIO({
data: {
requestedProjectName: projectName,
requestedCode: value.code,
requestedFileName: newFileName,
requestedFileNameWithExtension: newFileName,
},
})
}

View File

@ -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,

View File

@ -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

View File

@ -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 || '',
}

View File

@ -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,