Skip external imports in type generation

packages/app/src/cli/models/extensions/specifications/type-generation.test.ts

@@ -1,9 +1,13 @@
 import {
   createIntentsTypeDefinition,
   createToolsTypeDefinition,
+  findExplicitTsConfigFiles,
+  findAllImportedFiles,
   getGeneratedTypesHelperImportPath,
 } from './type-generation.js'
 import {AbortError} from '@shopify/cli-kit/node/error'
+import {inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
+import {joinPath, normalizePath} from '@shopify/cli-kit/node/path'
 import {describe, expect, test} from 'vitest'
 
 const adminGeneratedTypesHelperImportPath = '@shopify/ui-extensions/admin'
@@ -21,6 +25,191 @@ describe('getGeneratedTypesHelperImportPath', () => {
   })
 })
 
+describe('findAllImportedFiles', () => {
+  test('ignores commented-out imports', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const entryPath = joinPath(tmpDir, 'index.ts')
+      const commentedPath = joinPath(tmpDir, 'commented.ts')
+
+      await writeFile(
+        entryPath,
+        `
+          // import './commented.ts'
+          /*
+            import './commented.ts'
+          */
+        `,
+      )
+      await writeFile(commentedPath, `export const commented = true`)
+
+      const importedFiles = (await findAllImportedFiles(entryPath)).map((file) => normalizePath(file))
+
+      expect(importedFiles).not.toContain(normalizePath(commentedPath))
+    })
+  })
+
+  test('does not follow type-only imports and exports', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const entryPath = joinPath(tmpDir, 'index.ts')
+      const valuePath = joinPath(tmpDir, 'value.ts')
+      const valueNestedPath = joinPath(tmpDir, 'value-nested.ts')
+      const mixedValuePath = joinPath(tmpDir, 'mixed-value.ts')
+      const mixedExportPath = joinPath(tmpDir, 'mixed-export.ts')
+      const typePath = joinPath(tmpDir, 'types.ts')
+      const typeNestedPath = joinPath(tmpDir, 'type-nested.ts')
+      const exportedTypePath = joinPath(tmpDir, 'exported-types.ts')
+      const exportedTypeNestedPath = joinPath(tmpDir, 'exported-type-nested.ts')
+
+      await writeFile(
+        entryPath,
+        `
+          import './value.ts'
+          import MixedValue, { type MixedType } from './mixed-value.ts'
+          import type { TypeOnly } from './types.ts'
+          export { mixedValue, type MixedExportType } from './mixed-export.ts'
+          export type { ExportedTypeOnly } from './exported-types.ts'
+        `,
+      )
+      await writeFile(valuePath, `import './value-nested.ts'`)
+      await writeFile(valueNestedPath, `export const valueNested = true`)
+      await writeFile(mixedValuePath, `export default true; export type MixedType = string`)
+      await writeFile(mixedExportPath, `export const mixedValue = true; export type MixedExportType = string`)
+      await writeFile(typePath, `import './type-nested.ts'; export type TypeOnly = string`)
+      await writeFile(typeNestedPath, `export const typeNested = true`)
+      await writeFile(exportedTypePath, `import './exported-type-nested.ts'; export type ExportedTypeOnly = string`)
+      await writeFile(exportedTypeNestedPath, `export const exportedTypeNested = true`)
+
+      const importedFiles = (await findAllImportedFiles(entryPath)).map((file) => normalizePath(file))
+
+      expect(importedFiles).toContain(normalizePath(valuePath))
+      expect(importedFiles).toContain(normalizePath(valueNestedPath))
+      expect(importedFiles).toContain(normalizePath(mixedValuePath))
+      expect(importedFiles).toContain(normalizePath(mixedExportPath))
+      expect(importedFiles).not.toContain(normalizePath(typePath))
+      expect(importedFiles).not.toContain(normalizePath(typeNestedPath))
+      expect(importedFiles).not.toContain(normalizePath(exportedTypePath))
+      expect(importedFiles).not.toContain(normalizePath(exportedTypeNestedPath))
+    })
+  })
+
+  test('stops recursive import scanning at the boundary directory', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const extensionDir = joinPath(tmpDir, 'extensions', 'extension')
+      const srcDir = joinPath(extensionDir, 'src')
+      const sharedDir = joinPath(tmpDir, 'shared')
+
+      await mkdir(extensionDir)
+      await mkdir(srcDir)
+      await mkdir(sharedDir)
+
+      const entryPath = joinPath(srcDir, 'index.ts')
+      const localPath = joinPath(srcDir, 'local.ts')
+      const nestedPath = joinPath(srcDir, 'nested.ts')
+      const externalPath = joinPath(sharedDir, 'utils.ts')
+      const externalNestedPath = joinPath(sharedDir, 'secret.ts')
+
+      await writeFile(
+        entryPath,
+        `
+          import './local.ts'
+          import '../../../shared/utils.ts'
+        `,
+      )
+      await writeFile(localPath, `import './nested.ts'`)
+      await writeFile(nestedPath, `export const nested = true`)
+      await writeFile(externalPath, `import './secret.ts'`)
+      await writeFile(externalNestedPath, `export const secret = true`)
+
+      const importedFiles = (await findAllImportedFiles(entryPath, {boundaryDirectory: extensionDir})).map((file) =>
+        normalizePath(file),
+      )
+
+      expect(importedFiles).toContain(normalizePath(localPath))
+      expect(importedFiles).toContain(normalizePath(nestedPath))
+      expect(importedFiles).not.toContain(normalizePath(externalPath))
+      expect(importedFiles).not.toContain(normalizePath(externalNestedPath))
+    })
+  })
+
+  test('only follows files from an explicit tsconfig include list when provided', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const srcDir = joinPath(tmpDir, 'src')
+      const excludedDir = joinPath(tmpDir, 'excluded')
+
+      await mkdir(srcDir)
+      await mkdir(excludedDir)
+
+      const entryPath = joinPath(srcDir, 'index.ts')
+      const includedPath = joinPath(srcDir, 'included.ts')
+      const includedNestedPath = joinPath(srcDir, 'included-nested.ts')
+      const excludedPath = joinPath(excludedDir, 'excluded.ts')
+      const excludedNestedPath = joinPath(excludedDir, 'excluded-nested.ts')
+
+      await writeFile(
+        joinPath(tmpDir, 'tsconfig.json'),
+        JSON.stringify({
+          include: ['src/**/*'],
+        }),
+      )
+      await writeFile(
+        entryPath,
+        `
+          import './included.ts'
+          import '../excluded/excluded.ts'
+        `,
+      )
+      await writeFile(includedPath, `import './included-nested.ts'`)
+      await writeFile(includedNestedPath, `export const includedNested = true`)
+      await writeFile(excludedPath, `import './excluded-nested.ts'`)
+      await writeFile(excludedNestedPath, `export const excludedNested = true`)
+
+      const allowedFiles = await findExplicitTsConfigFiles(entryPath, tmpDir)
+      const importedFiles = (
+        await findAllImportedFiles(entryPath, {
+          boundaryDirectory: tmpDir,
+          allowedFiles,
+          alwaysAllowedFiles: new Set([entryPath]),
+        })
+      ).map((file) => normalizePath(file))
+
+      expect(importedFiles).toContain(normalizePath(includedPath))
+      expect(importedFiles).toContain(normalizePath(includedNestedPath))
+      expect(importedFiles).not.toContain(normalizePath(excludedPath))
+      expect(importedFiles).not.toContain(normalizePath(excludedNestedPath))
+    })
+  })
+
+  test('does not follow declaration files', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const entryPath = joinPath(tmpDir, 'index.ts')
+      const declarationPath = joinPath(tmpDir, 'types.d.ts')
+      const nestedPath = joinPath(tmpDir, 'nested.ts')
+
+      await writeFile(entryPath, `import './types.d.ts'`)
+      await writeFile(declarationPath, `import './nested.ts'`)
+      await writeFile(nestedPath, `export const nested = true`)
+
+      const importedFiles = (await findAllImportedFiles(entryPath, {boundaryDirectory: tmpDir})).map((file) =>
+        normalizePath(file),
+      )
+
+      expect(importedFiles).not.toContain(normalizePath(declarationPath))
+      expect(importedFiles).not.toContain(normalizePath(nestedPath))
+    })
+  })
+
+  test('does not use a tsconfig allowlist when files and include are implicit', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      const entryPath = joinPath(tmpDir, 'index.ts')
+
+      await writeFile(joinPath(tmpDir, 'tsconfig.json'), '{}')
+      await writeFile(entryPath, `export const entry = true`)
+
+      await expect(findExplicitTsConfigFiles(entryPath, tmpDir)).resolves.toBeUndefined()
+    })
+  })
+})
+
 describe('createIntentsTypeDefinition', () => {
   test('returns empty string when intents array is empty', async () => {
     // When

packages/app/src/cli/models/extensions/specifications/type-generation.ts

@@ -1,5 +1,5 @@
 import {fileExists, findPathUp, readFileSync} from '@shopify/cli-kit/node/fs'
-import {dirname, joinPath, relativizePath, resolvePath} from '@shopify/cli-kit/node/path'
+import {dirname, isSubpath, joinPath, relativizePath, resolvePath} from '@shopify/cli-kit/node/path'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {compile} from 'json-schema-to-typescript'
 import {pascalize} from '@shopify/cli-kit/common/string'
@@ -46,23 +46,27 @@ export function parseApiVersion(apiVersion: string): {year: number; month: numbe
   return {year: parseInt(year, 10), month: parseInt(month, 10)}
 }
 
-async function loadTsConfig(
-  startPath: string,
-): Promise<{compilerOptions: ts.CompilerOptions; configPath: string | undefined}> {
+async function loadTsConfig(startPath: string): Promise<{
+  compilerOptions: ts.CompilerOptions
+  configPath: string | undefined
+  fileNames?: string[]
+  hasExplicitFiles: boolean
+}> {
   const ts = await loadTypeScript()
   const configPath = ts.findConfigFile(startPath, ts.sys.fileExists.bind(ts.sys), 'tsconfig.json')
   if (!configPath) {
-    return {compilerOptions: {}, configPath: undefined}
+    return {compilerOptions: {}, configPath: undefined, hasExplicitFiles: false}
   }
 
   const configFile = ts.readConfigFile(configPath, ts.sys.readFile.bind(ts.sys))
   if (configFile.error) {
-    return {compilerOptions: {}, configPath}
+    return {compilerOptions: {}, configPath, hasExplicitFiles: false}
   }
 
   const parsedConfig = ts.parseJsonConfigFileContent(configFile.config, ts.sys, dirname(configPath))
+  const hasExplicitFiles = Boolean(configFile.config.files ?? configFile.config.include)
 
-  return {compilerOptions: parsedConfig.options, configPath}
+  return {compilerOptions: parsedConfig.options, configPath, fileNames: parsedConfig.fileNames, hasExplicitFiles}
 }
 
 async function fallbackResolve(importPath: string, baseDir: string): Promise<string | null> {
@@ -95,8 +99,39 @@ async function fallbackResolve(importPath: string, baseDir: string): Promise<str
   return null
 }
 
-async function parseAndResolveImports(filePath: string): Promise<string[]> {
+interface FindAllImportedFilesOptions {
+  boundaryDirectory?: string
+  allowedFiles?: Set<string>
+  alwaysAllowedFiles?: Set<string>
+}
+
+function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean {
+  if (!boundaryDirectory) return true
+  return isSubpath(resolvePath(boundaryDirectory), resolvePath(filePath))
+}
+
+function isAllowedFile(filePath: string, options: FindAllImportedFilesOptions): boolean {
+  if (!options.allowedFiles) return true
+
+  const resolvedPath = resolvePath(filePath)
+  return options.allowedFiles.has(resolvedPath) || Boolean(options.alwaysAllowedFiles?.has(resolvedPath))
+}
+
+function isScannableFile(filePath: string, options: FindAllImportedFilesOptions): boolean {
+  return (
+    !filePath.includes('node_modules') &&
+    !filePath.endsWith('.d.ts') &&
+    isWithinBoundary(filePath, options.boundaryDirectory) &&
+    isAllowedFile(filePath, options)
+  )
+}
+
+async function parseAndResolveImports(filePath: string, options: FindAllImportedFilesOptions = {}): Promise<string[]> {
   try {
+    if (!isAllowedFile(filePath, options)) {
+      return []
+    }
+
     const ts = await loadTypeScript()
     const content = readFileSync(filePath).toString()
     const resolvedPaths: string[] = []
@@ -119,13 +154,38 @@ async function parseAndResolveImports(filePath: string): Promise<string[]> {
 
     const visit = (node: ts.Node): void => {
       if (ts.isImportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier)) {
+        if (node.importClause?.isTypeOnly) {
+          return
+        }
+
+        if (
+          !node.importClause?.name &&
+          node.importClause?.namedBindings &&
+          ts.isNamedImports(node.importClause.namedBindings) &&
+          node.importClause.namedBindings.elements.every((element) => element.isTypeOnly)
+        ) {
+          return
+        }
+
         importPaths.push(node.moduleSpecifier.text)
       } else if (ts.isCallExpression(node) && node.expression.kind === ts.SyntaxKind.ImportKeyword) {
         const firstArg = node.arguments[0]
         if (firstArg && ts.isStringLiteral(firstArg)) {
           importPaths.push(firstArg.text)
         }
       } else if (ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier)) {
+        if (node.isTypeOnly) {
+          return
+        }
+
+        if (
+          node.exportClause &&
+          ts.isNamedExports(node.exportClause) &&
+          node.exportClause.elements.every((element) => element.isTypeOnly)
+        ) {
+          return
+        }
+
         importPaths.push(node.moduleSpecifier.text)
       }
 
@@ -147,14 +207,14 @@ async function parseAndResolveImports(filePath: string): Promise<string[]> {
       if (resolvedModule.resolvedModule?.resolvedFileName) {
         const resolvedPath = resolvedModule.resolvedModule.resolvedFileName
 
-        if (!resolvedPath.includes('node_modules')) {
+        if (isScannableFile(resolvedPath, options)) {
           resolvedPaths.push(resolvedPath)
         }
       } else {
         // Fallback to manual resolution for edge cases
         // eslint-disable-next-line no-await-in-loop
         const fallbackPath = await fallbackResolve(importPath, dirname(filePath))
-        if (fallbackPath) {
+        if (fallbackPath && isScannableFile(fallbackPath, options)) {
           resolvedPaths.push(fallbackPath)
         }
       }
@@ -170,26 +230,46 @@ async function parseAndResolveImports(filePath: string): Promise<string[]> {
   }
 }
 
-export async function findAllImportedFiles(filePath: string, visited = new Set<string>()): Promise<string[]> {
+export async function findAllImportedFiles(
+  filePath: string,
+  options: FindAllImportedFilesOptions = {},
+  visited = new Set<string>(),
+): Promise<string[]> {
   if (visited.has(filePath)) {
     return []
   }
 
   visited.add(filePath)
-  const resolvedPaths = await parseAndResolveImports(filePath)
+  const resolvedPaths = await parseAndResolveImports(filePath, options)
 
   const allFiles = [...resolvedPaths]
 
   // Recursively find imports from the resolved files
   for (const resolvedPath of resolvedPaths) {
     // eslint-disable-next-line no-await-in-loop
-    const nestedImports = await findAllImportedFiles(resolvedPath, visited)
+    const nestedImports = await findAllImportedFiles(resolvedPath, options, visited)
     allFiles.push(...nestedImports)
   }
 
   return uniq(allFiles)
 }
 
+export async function findExplicitTsConfigFiles(
+  fromFile: string,
+  extensionDirectory: string,
+): Promise<Set<string> | undefined> {
+  const {configPath, fileNames, hasExplicitFiles} = await loadTsConfig(fromFile)
+  if (!configPath || !hasExplicitFiles) return
+
+  if (!isWithinBoundary(configPath, extensionDirectory)) return
+
+  return new Set(
+    (fileNames ?? [])
+      .filter((fileName) => isWithinBoundary(fileName, extensionDirectory))
+      .map((fileName) => resolvePath(fileName)),
+  )
+}
+
 interface CreateTypeDefinitionOptions {
   fullPath: string
   typeFilePath: string