Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -694,3 +694,31 @@ describe('rescanImports', async () => {
})
})
})

describe('SHOPIFY_CLI_DISABLE_IMPORT_SCANNING', () => {
test('skips import scanning when env var is set', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const extensionInstance = await testUIExtension({
directory: tmpDir,
entrySourceFilePath: joinPath(tmpDir, 'src', 'index.ts'),
})

const srcDir = joinPath(tmpDir, 'src')
await mkdir(srcDir)
await writeFile(joinPath(srcDir, 'index.ts'), 'import "../shared"')

vi.mocked(extractImportPathsRecursively).mockReset()
vi.mocked(extractImportPathsRecursively).mockReturnValue(['/some/external/file.ts'])

process.env.SHOPIFY_CLI_DISABLE_IMPORT_SCANNING = '1'
try {
const watched = extensionInstance.watchedFiles()
expect(extractImportPathsRecursively).not.toHaveBeenCalled()
expect(watched.some((file) => file.includes('external'))).toBe(false)
} finally {
delete process.env.SHOPIFY_CLI_DISABLE_IMPORT_SCANNING
vi.mocked(extractImportPathsRecursively).mockReset()
}
})
})
})
28 changes: 24 additions & 4 deletions packages/app/src/cli/models/extensions/extension-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ import {joinPath, basename, normalizePath, resolvePath} from '@shopify/cli-kit/n
import {fileExists, touchFile, moveFile, writeFile, glob, copyFile, globSync} from '@shopify/cli-kit/node/fs'
import {getPathValue} from '@shopify/cli-kit/common/object'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {extractJSImports, extractImportPathsRecursively} from '@shopify/cli-kit/node/import-extractor'
import {
extractJSImports,
extractImportPathsRecursively,
clearImportPathsCache,
getImportScanningCacheStats,
} from '@shopify/cli-kit/node/import-extractor'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {uniq} from '@shopify/cli-kit/common/array'

export const CONFIG_EXTENSION_IDS: string[] = [
Expand Down Expand Up @@ -516,6 +522,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
async rescanImports(): Promise<boolean> {
const oldImportPaths = this.cachedImportPaths
this.cachedImportPaths = undefined
clearImportPathsCache()
this.scanImports()
return oldImportPaths !== this.cachedImportPaths
}
Expand All @@ -530,13 +537,26 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
return this.cachedImportPaths
}

if (isTruthy(process.env.SHOPIFY_CLI_DISABLE_IMPORT_SCANNING)) {
this.cachedImportPaths = []
return this.cachedImportPaths
}

try {
const imports = this.devSessionDefaultWatchPaths().flatMap((entryFile) => {
const startTime = performance.now()
const entryFiles = this.devSessionDefaultWatchPaths()

const imports = entryFiles.flatMap((entryFile) => {
return extractImportPathsRecursively(entryFile).map((importPath) => normalizePath(resolvePath(importPath)))
})
// Cache and return unique paths

this.cachedImportPaths = uniq(imports) ?? []
outputDebug(`Found ${this.cachedImportPaths.length} external imports (recursively) for extension ${this.handle}`)
const elapsed = Math.round(performance.now() - startTime)
const cacheStats = getImportScanningCacheStats()
const cacheInfo = cacheStats ? ` (cache: ${cacheStats.directImports} parsed, ${cacheStats.fileExists} stats)` : ''
outputDebug(
`Import scan for "${this.handle}": ${entryFiles.length} entries, ${this.cachedImportPaths.length} files, ${elapsed}ms${cacheInfo}`,
)
return this.cachedImportPaths
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions packages/cli-kit/src/private/node/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const environmentVariables = {
neverUsePartnersApi: 'SHOPIFY_CLI_NEVER_USE_PARTNERS_API',
skipNetworkLevelRetry: 'SHOPIFY_CLI_SKIP_NETWORK_LEVEL_RETRY',
maxRequestTimeForNetworkCalls: 'SHOPIFY_CLI_MAX_REQUEST_TIME_FOR_NETWORK_CALLS',
disableImportScanning: 'SHOPIFY_CLI_DISABLE_IMPORT_SCANNING',
}

export const defaultThemeKitAccessDomain = 'theme-kit-access.shopifyapps.com'
Expand Down
89 changes: 87 additions & 2 deletions packages/cli-kit/src/public/node/import-extractor.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import {inTemporaryDirectory, mkdir, writeFile} from './fs.js'
import {joinPath} from './path.js'
import {extractImportPaths, extractImportPathsRecursively} from './import-extractor.js'
import {describe, test, expect} from 'vitest'
import {
extractImportPaths,
extractImportPathsRecursively,
clearImportPathsCache,
getImportScanningCacheStats,
} from './import-extractor.js'
import {describe, test, expect, beforeEach} from 'vitest'

beforeEach(() => {
clearImportPathsCache()
})

describe('extractImportPaths', () => {
describe('JavaScript imports', () => {
Expand Down Expand Up @@ -610,3 +619,79 @@ describe('extractImportPathsRecursively', () => {
})
})
})

describe('clearImportPathsCache', () => {
test('picks up file changes after cache is cleared', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const mainFile = joinPath(tmpDir, 'main.js')
const utilsFile = joinPath(tmpDir, 'utils.js')

await writeFile(utilsFile, 'export const foo = "bar"')
await writeFile(mainFile, `import { foo } from './utils.js'`)

const firstResult = extractImportPaths(mainFile)
expect(firstResult).toContain(utilsFile)

// Modify the file to add a new import
const helpersFile = joinPath(tmpDir, 'helpers.js')
await writeFile(helpersFile, 'export const helper = () => {}')
await writeFile(mainFile, `import { foo } from './utils.js'\nimport { helper } from './helpers.js'`)

// Without clearing, we still get cached results
const cachedResult = extractImportPaths(mainFile)
expect(cachedResult).toHaveLength(1)

// After clearing, new imports are picked up
clearImportPathsCache()
const freshResult = extractImportPaths(mainFile)
expect(freshResult).toContain(utilsFile)
expect(freshResult).toContain(helpersFile)
expect(freshResult).toHaveLength(2)
})
})

test('clears all caches including filesystem stat caches', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const mainFile = joinPath(tmpDir, 'main.js')
const utilsFile = joinPath(tmpDir, 'utils.js')

await writeFile(utilsFile, 'export const foo = "bar"')
await writeFile(mainFile, `import { foo } from './utils.js'`)

extractImportPaths(mainFile)
const statsAfterScan = getImportScanningCacheStats()
expect(statsAfterScan.directImports).toBeGreaterThan(0)
expect(statsAfterScan.fileExists).toBeGreaterThan(0)

clearImportPathsCache()
const statsAfterClear = getImportScanningCacheStats()
expect(statsAfterClear.directImports).toBe(0)
expect(statsAfterClear.fileExists).toBe(0)
expect(statsAfterClear.isDir).toBe(0)
})
})
})

describe('getImportScanningCacheStats', () => {
test('tracks cache sizes across multiple scans', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const file1 = joinPath(tmpDir, 'file1.js')
const file2 = joinPath(tmpDir, 'file2.js')
const shared = joinPath(tmpDir, 'shared.js')

await writeFile(shared, 'export const x = 1')
await writeFile(file1, `import { x } from './shared.js'`)
await writeFile(file2, `import { x } from './shared.js'`)

extractImportPathsRecursively(file1)
const statsAfterFirst = getImportScanningCacheStats()

extractImportPathsRecursively(file2)
const statsAfterSecond = getImportScanningCacheStats()

// Second scan should grow the directImports cache (file2 is new)
// but shared.js stat results are reused from the first scan
expect(statsAfterSecond.directImports).toBeGreaterThanOrEqual(statsAfterFirst.directImports)
})
})
})
91 changes: 70 additions & 21 deletions packages/cli-kit/src/public/node/import-extractor.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,76 @@
import {readFileSync, fileExistsSync, isDirectorySync} from './fs.js'
import {dirname, joinPath} from './path.js'

// Caches direct import results per file path to avoid redundant file reads and parsing
// when multiple extensions import the same shared code.
const directImportsCache = new Map<string, string[]>()

// Caches filesystem stat results to avoid redundant synchronous I/O.
// Each stat call also triggers outputDebug overhead, so caching here
// avoids both the kernel round-trip and the debug string construction.
const fileExistsCache = new Map<string, boolean>()
const isDirCache = new Map<string, boolean>()

function cachedFileExists(path: string): boolean {
const cached = fileExistsCache.get(path)
if (cached !== undefined) return cached
const result = fileExistsSync(path)
fileExistsCache.set(path, result)
return result
}

function cachedIsDir(path: string): boolean {
const cached = isDirCache.get(path)
if (cached !== undefined) return cached
const result = isDirectorySync(path)
isDirCache.set(path, result)
return result
}

/**
* Clears all import-scanning caches (direct imports, recursive results, and filesystem stats).
* Should be called when watched files change so that rescanning picks up updated imports.
*/
export function clearImportPathsCache(): void {
directImportsCache.clear()
fileExistsCache.clear()
isDirCache.clear()
}

/**
* Extracts import paths from a source file.
* Supports JavaScript, TypeScript, and Rust files.
* Results are cached per file path to avoid redundant I/O.
*
* @param filePath - Path to the file to analyze.
* @returns Array of absolute paths to imported files.
*/
export function extractImportPaths(filePath: string): string[] {
const cached = directImportsCache.get(filePath)
if (cached) return cached

const content = readFileSync(filePath).toString()
const ext = filePath.substring(filePath.lastIndexOf('.'))

let result: string[]
switch (ext) {
case '.js':
case '.mjs':
case '.cjs':
case '.ts':
case '.tsx':
case '.jsx':
return extractJSLikeImports(content, filePath)
result = extractJSLikeImports(content, filePath)
break
case '.rs':
return extractRustImports(content, filePath)
result = extractRustImports(content, filePath)
break
default:
return []
result = []
}

directImportsCache.set(filePath, result)
return result
}

/**
Expand All @@ -38,40 +84,46 @@ export function extractImportPaths(filePath: string): string[] {
* @throws If an unexpected error occurs while processing files (not including ENOENT file not found errors).
*/
export function extractImportPathsRecursively(filePath: string, visited: Set<string> = new Set<string>()): string[] {
// Avoid processing the same file twice (handles circular dependencies)
if (visited.has(filePath)) {
return []
}

visited.add(filePath)

// Get direct imports from this file
const directImports = extractImportPaths(filePath)
const allImports = [filePath, ...directImports]

// Recursively process each imported file
for (const importedFile of directImports) {
try {
// Only process files that exist and are not directories
// Note: resolveJSImport already resolves directory imports to their index files
if (fileExistsSync(importedFile) && !isDirectorySync(importedFile)) {
if (cachedFileExists(importedFile) && !cachedIsDir(importedFile)) {
const nestedImports = extractImportPathsRecursively(importedFile, visited)
allImports.push(...nestedImports)
}
} catch (error) {
// Rethrow unexpected errors after checking for expected file read errors
if (error instanceof Error && error.message.includes('ENOENT')) {
// Skip files that don't exist or can't be read
continue
}
throw error
}
}

// Return unique list of imports
return [...new Set(allImports)]
}

/**
* Returns diagnostic information about the import scanning caches.
* Useful for debugging performance issues with --verbose.
*
* @returns Cache size stats for directImports, fileExists, and isDir.
*/
export function getImportScanningCacheStats(): {directImports: number; fileExists: number; isDir: number} {
return {
directImports: directImportsCache.size,
fileExists: fileExistsCache.size,
isDir: isDirCache.size,
}
}

/**
* Extracts import paths from a JavaScript content.
*
Expand Down Expand Up @@ -139,7 +191,7 @@ function extractRustImports(content: string, filePath: string): string[] {
const pathValue = match[1]
if (pathValue) {
const resolvedPath = joinPath(dirname(filePath), pathValue)
if (fileExistsSync(resolvedPath)) {
if (cachedFileExists(resolvedPath)) {
imports.push(resolvedPath)
}
}
Expand All @@ -149,11 +201,10 @@ function extractRustImports(content: string, filePath: string): string[] {
}

function resolveJSImport(importPath: string, fromFile: string): string | null {
const basePath = fileExistsSync(fromFile) && isDirectorySync(fromFile) ? fromFile : dirname(fromFile)
const basePath = cachedFileExists(fromFile) && cachedIsDir(fromFile) ? fromFile : dirname(fromFile)
const resolvedPath = joinPath(basePath, importPath)

// If the import path resolves to a directory, look for index files
if (fileExistsSync(resolvedPath) && isDirectorySync(resolvedPath)) {
if (cachedFileExists(resolvedPath) && cachedIsDir(resolvedPath)) {
const indexPaths = [
joinPath(resolvedPath, 'index.js'),
joinPath(resolvedPath, 'index.ts'),
Expand All @@ -162,15 +213,13 @@ function resolveJSImport(importPath: string, fromFile: string): string | null {
]

for (const indexPath of indexPaths) {
if (fileExistsSync(indexPath) && !isDirectorySync(indexPath)) {
if (cachedFileExists(indexPath) && !cachedIsDir(indexPath)) {
return indexPath
}
}
// If no index file found, don't return the directory
return null
}

// Check for file with extensions
const possiblePaths = [
resolvedPath,
`${resolvedPath}.js`,
Expand All @@ -180,7 +229,7 @@ function resolveJSImport(importPath: string, fromFile: string): string | null {
]

for (const path of possiblePaths) {
if (fileExistsSync(path) && !isDirectorySync(path)) {
if (cachedFileExists(path) && !cachedIsDir(path)) {
return path
}
}
Expand All @@ -193,7 +242,7 @@ function resolveRustModule(modName: string, fromFile: string): string | null {
const possiblePaths = [joinPath(basePath, `${modName}.rs`), joinPath(basePath, modName, 'mod.rs')]

for (const path of possiblePaths) {
if (fileExistsSync(path)) {
if (cachedFileExists(path)) {
return path
}
}
Expand Down
Loading