From 53cc39b17a2979931ece1b208d356a9ca1affb2a Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 8 Jun 2026 11:48:15 +0200 Subject: [PATCH 1/5] Skip external imports in type generation --- .../specifications/type-generation.test.ts | 44 +++++++++++++++++++ .../specifications/type-generation.ts | 27 +++++++++--- .../extensions/specifications/ui_extension.ts | 6 ++- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts index b3029f3599e..f6871671d62 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts @@ -1,9 +1,12 @@ import { createIntentsTypeDefinition, createToolsTypeDefinition, + 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 +24,47 @@ describe('getGeneratedTypesHelperImportPath', () => { }) }) +describe('findAllImportedFiles', () => { + 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)) + }) + }) +}) + describe('createIntentsTypeDefinition', () => { test('returns empty string when intents array is empty', async () => { // When diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.ts index 32da0e44a6d..3f3cfcf47d2 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.ts +++ b/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' @@ -95,7 +95,16 @@ async function fallbackResolve(importPath: string, baseDir: string): Promise { +interface FindAllImportedFilesOptions { + boundaryDirectory?: string +} + +function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean { + if (!boundaryDirectory) return true + return isSubpath(resolvePath(boundaryDirectory), resolvePath(filePath)) +} + +async function parseAndResolveImports(filePath: string, options: FindAllImportedFilesOptions = {}): Promise { try { const ts = await loadTypeScript() const content = readFileSync(filePath).toString() @@ -147,14 +156,14 @@ async function parseAndResolveImports(filePath: string): Promise { if (resolvedModule.resolvedModule?.resolvedFileName) { const resolvedPath = resolvedModule.resolvedModule.resolvedFileName - if (!resolvedPath.includes('node_modules')) { + if (!resolvedPath.includes('node_modules') && isWithinBoundary(resolvedPath, options.boundaryDirectory)) { 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 && isWithinBoundary(fallbackPath, options.boundaryDirectory)) { resolvedPaths.push(fallbackPath) } } @@ -170,20 +179,24 @@ async function parseAndResolveImports(filePath: string): Promise { } } -export async function findAllImportedFiles(filePath: string, visited = new Set()): Promise { +export async function findAllImportedFiles( + filePath: string, + options: FindAllImportedFilesOptions = {}, + visited = new Set(), +): Promise { 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) } diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 594ed9e27cc..130286e6ebd 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -254,7 +254,7 @@ const uiExtensionSpec = createExtensionSpecification({ if (!exists) continue // Find all imported files recursively - const importedFiles = await findAllImportedFiles(fullPath) + const importedFiles = await findAllImportedFiles(fullPath, {boundaryDirectory: extension.directory}) // Associate imported files with this extension point's target for (const importedFile of importedFiles) { @@ -271,7 +271,9 @@ const uiExtensionSpec = createExtensionSpecification({ ) const shouldRenderExists = await fileExists(shouldRenderPath) if (shouldRenderExists) { - const shouldRenderImports = await findAllImportedFiles(shouldRenderPath) + const shouldRenderImports = await findAllImportedFiles(shouldRenderPath, { + boundaryDirectory: extension.directory, + }) for (const importedFile of shouldRenderImports) { const currentTargets = fileToTargetsMap.get(importedFile) ?? [] currentTargets.push(getShouldRenderTarget(extensionPoint.target)) From 41ccb2a936879d2952ae621fe2743d61ce2fe69d Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 9 Jun 2026 17:52:39 +0200 Subject: [PATCH 2/5] Do not follow type imports --- .../specifications/type-generation.test.ts | 66 +++++++++++++++++++ .../specifications/type-generation.ts | 25 +++++++ 2 files changed, 91 insertions(+) diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts index f6871671d62..4781a82353b 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts @@ -25,6 +25,72 @@ 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') diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.ts index 3f3cfcf47d2..61d62406d0c 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.ts @@ -128,6 +128,19 @@ async function parseAndResolveImports(filePath: string, options: FindAllImported 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] @@ -135,6 +148,18 @@ async function parseAndResolveImports(filePath: string, options: FindAllImported 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) } From 92f245fe3637d06e35633a3eee8e7c7d14024c61 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 10 Jun 2026 11:24:17 +0200 Subject: [PATCH 3/5] Use tsconfig files when provided --- .../specifications/type-generation.test.ts | 79 +++++++++++++++++++ .../specifications/type-generation.ts | 58 ++++++++++++-- .../extensions/specifications/ui_extension.ts | 11 ++- 3 files changed, 139 insertions(+), 9 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts index 4781a82353b..ee74e8e5d7c 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts @@ -1,6 +1,7 @@ import { createIntentsTypeDefinition, createToolsTypeDefinition, + findExplicitTsConfigFiles, findAllImportedFiles, getGeneratedTypesHelperImportPath, } from './type-generation.js' @@ -129,6 +130,84 @@ describe('findAllImportedFiles', () => { 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', () => { diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.ts index 61d62406d0c..4a36fffddc6 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.ts @@ -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 { @@ -97,6 +101,8 @@ async function fallbackResolve(importPath: string, baseDir: string): Promise + alwaysAllowedFiles?: Set } function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean { @@ -104,8 +110,28 @@ function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean 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 { try { + if (!isAllowedFile(filePath, options)) { + return [] + } + const ts = await loadTypeScript() const content = readFileSync(filePath).toString() const resolvedPaths: string[] = [] @@ -181,14 +207,14 @@ async function parseAndResolveImports(filePath: string, options: FindAllImported if (resolvedModule.resolvedModule?.resolvedFileName) { const resolvedPath = resolvedModule.resolvedModule.resolvedFileName - if (!resolvedPath.includes('node_modules') && isWithinBoundary(resolvedPath, options.boundaryDirectory)) { + 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 && isWithinBoundary(fallbackPath, options.boundaryDirectory)) { + if (fallbackPath && isScannableFile(fallbackPath, options)) { resolvedPaths.push(fallbackPath) } } @@ -228,6 +254,22 @@ export async function findAllImportedFiles( return uniq(allFiles) } +export async function findExplicitTsConfigFiles( + fromFile: string, + extensionDirectory: string, +): Promise | 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 diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 130286e6ebd..75a076872a7 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -4,6 +4,7 @@ import { createTypeDefinition, createToolsTypeDefinition, findNearestTsConfigDir, + findExplicitTsConfigFiles, getGeneratedTypesHelperImportPath, IntentSchemaFileSchema, parseApiVersion, @@ -254,7 +255,12 @@ const uiExtensionSpec = createExtensionSpecification({ if (!exists) continue // Find all imported files recursively - const importedFiles = await findAllImportedFiles(fullPath, {boundaryDirectory: extension.directory}) + const allowedFiles = await findExplicitTsConfigFiles(fullPath, extension.directory) + const importedFiles = await findAllImportedFiles(fullPath, { + boundaryDirectory: extension.directory, + allowedFiles, + alwaysAllowedFiles: new Set([fullPath]), + }) // Associate imported files with this extension point's target for (const importedFile of importedFiles) { @@ -271,8 +277,11 @@ const uiExtensionSpec = createExtensionSpecification({ ) const shouldRenderExists = await fileExists(shouldRenderPath) if (shouldRenderExists) { + const shouldRenderAllowedFiles = await findExplicitTsConfigFiles(shouldRenderPath, extension.directory) const shouldRenderImports = await findAllImportedFiles(shouldRenderPath, { boundaryDirectory: extension.directory, + allowedFiles: shouldRenderAllowedFiles, + alwaysAllowedFiles: new Set([shouldRenderPath]), }) for (const importedFile of shouldRenderImports) { const currentTargets = fileToTargetsMap.get(importedFile) ?? [] From e3e7195661a3c5a38979e3559189328afda91713 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 11 Jun 2026 12:39:50 +0200 Subject: [PATCH 4/5] Add import cache to avoid repetition --- .../specifications/type-generation.test.ts | 30 ++++++++++++++++++- .../specifications/type-generation.ts | 27 ++++++++++++----- .../extensions/specifications/ui_extension.ts | 3 ++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts index ee74e8e5d7c..60b2f2bdb64 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts @@ -5,10 +5,11 @@ import { findAllImportedFiles, getGeneratedTypesHelperImportPath, } from './type-generation.js' +import * as fs from '@shopify/cli-kit/node/fs' 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' +import {describe, expect, test, vi} from 'vitest' const adminGeneratedTypesHelperImportPath = '@shopify/ui-extensions/admin' @@ -208,6 +209,33 @@ describe('findAllImportedFiles', () => { await expect(findExplicitTsConfigFiles(entryPath, tmpDir)).resolves.toBeUndefined() }) }) + + test('uses cached imports when the same file is scanned more than once', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const entryPath = joinPath(tmpDir, 'index.ts') + const localPath = joinPath(tmpDir, 'local.ts') + const nestedPath = joinPath(tmpDir, 'nested.ts') + + await writeFile(entryPath, `import './local.ts'`) + await writeFile(localPath, `import './nested.ts'`) + await writeFile(nestedPath, `export const nested = true`) + + const readFileSync = vi.spyOn(fs, 'readFileSync') + const importCache = new Map() + + await findAllImportedFiles(entryPath, {boundaryDirectory: tmpDir, importCache}) + await findAllImportedFiles(entryPath, {boundaryDirectory: tmpDir, importCache}) + + const readCountsByPath = new Map() + for (const [path] of readFileSync.mock.calls) { + readCountsByPath.set(path, (readCountsByPath.get(path) ?? 0) + 1) + } + + expect(readCountsByPath.get(entryPath)).toBe(1) + expect(readCountsByPath.get(localPath)).toBe(1) + expect(readCountsByPath.get(nestedPath)).toBe(1) + }) + }) }) describe('createIntentsTypeDefinition', () => { diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.ts index 4a36fffddc6..56de6d60e65 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.ts @@ -103,6 +103,11 @@ interface FindAllImportedFilesOptions { boundaryDirectory?: string allowedFiles?: Set alwaysAllowedFiles?: Set + importCache?: Map +} + +interface ParseAndResolveImportsOptions { + importCache?: Map } function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean { @@ -126,10 +131,15 @@ function isScannableFile(filePath: string, options: FindAllImportedFilesOptions) ) } -async function parseAndResolveImports(filePath: string, options: FindAllImportedFilesOptions = {}): Promise { +async function parseAndResolveImports( + filePath: string, + options: ParseAndResolveImportsOptions = {}, +): Promise { try { - if (!isAllowedFile(filePath, options)) { - return [] + const resolvedFilePath = resolvePath(filePath) + const cachedImports = options.importCache?.get(resolvedFilePath) + if (cachedImports) { + return cachedImports } const ts = await loadTypeScript() @@ -207,19 +217,18 @@ async function parseAndResolveImports(filePath: string, options: FindAllImported if (resolvedModule.resolvedModule?.resolvedFileName) { const resolvedPath = resolvedModule.resolvedModule.resolvedFileName - if (isScannableFile(resolvedPath, options)) { - resolvedPaths.push(resolvedPath) - } + 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 && isScannableFile(fallbackPath, options)) { + if (fallbackPath) { resolvedPaths.push(fallbackPath) } } } + options.importCache?.set(resolvedFilePath, resolvedPaths) return resolvedPaths } catch (error) { // Re-throw AbortError as-is, wrap other errors @@ -240,7 +249,9 @@ export async function findAllImportedFiles( } visited.add(filePath) - const resolvedPaths = await parseAndResolveImports(filePath, options) + const resolvedPaths = (await parseAndResolveImports(filePath, options)).filter((resolvedPath) => + isScannableFile(resolvedPath, options), + ) const allFiles = [...resolvedPaths] diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index 75a076872a7..d6e0059b0f1 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -211,6 +211,7 @@ const uiExtensionSpec = createExtensionSpecification({ const fileToTargetsMap = new Map() const fileToToolsMap = new Map() const fileToIntentsMap = new Map>() + const importCache = new Map() // First pass: collect all entry point files and their targets for await (const extensionPoint of configuration.extension_points) { @@ -260,6 +261,7 @@ const uiExtensionSpec = createExtensionSpecification({ boundaryDirectory: extension.directory, allowedFiles, alwaysAllowedFiles: new Set([fullPath]), + importCache, }) // Associate imported files with this extension point's target @@ -282,6 +284,7 @@ const uiExtensionSpec = createExtensionSpecification({ boundaryDirectory: extension.directory, allowedFiles: shouldRenderAllowedFiles, alwaysAllowedFiles: new Set([shouldRenderPath]), + importCache, }) for (const importedFile of shouldRenderImports) { const currentTargets = fileToTargetsMap.get(importedFile) ?? [] From 2049a1d773d5ba0109e06a5ef07c86d61b74455f Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 3 Jul 2026 12:24:30 +0200 Subject: [PATCH 5/5] Address type generation review feedback --- .../skip-external-type-generation-imports.md | 5 ++ .../specifications/type-generation.test.ts | 65 +++++++++++++++++++ .../specifications/type-generation.ts | 36 ++++++++-- .../extensions/specifications/ui_extension.ts | 16 +++-- 4 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 .changeset/skip-external-type-generation-imports.md diff --git a/.changeset/skip-external-type-generation-imports.md b/.changeset/skip-external-type-generation-imports.md new file mode 100644 index 00000000000..b7360155bb5 --- /dev/null +++ b/.changeset/skip-external-type-generation-imports.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli': patch +--- + +Improve extension type generation by avoiding unnecessary scans outside extension source files. diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts index 60b2f2bdb64..4593c539bf8 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.test.ts @@ -60,14 +60,20 @@ describe('findAllImportedFiles', () => { const typeNestedPath = joinPath(tmpDir, 'type-nested.ts') const exportedTypePath = joinPath(tmpDir, 'exported-types.ts') const exportedTypeNestedPath = joinPath(tmpDir, 'exported-type-nested.ts') + const emptyImportPath = joinPath(tmpDir, 'empty-import.ts') + const emptyImportNestedPath = joinPath(tmpDir, 'empty-import-nested.ts') + const emptyExportPath = joinPath(tmpDir, 'empty-export.ts') + const emptyExportNestedPath = joinPath(tmpDir, 'empty-export-nested.ts') await writeFile( entryPath, ` import './value.ts' import MixedValue, { type MixedType } from './mixed-value.ts' + import {} from './empty-import.ts' import type { TypeOnly } from './types.ts' export { mixedValue, type MixedExportType } from './mixed-export.ts' + export {} from './empty-export.ts' export type { ExportedTypeOnly } from './exported-types.ts' `, ) @@ -79,6 +85,10 @@ describe('findAllImportedFiles', () => { 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`) + await writeFile(emptyImportPath, `import './empty-import-nested.ts'`) + await writeFile(emptyImportNestedPath, `export const emptyImportNested = true`) + await writeFile(emptyExportPath, `import './empty-export-nested.ts'`) + await writeFile(emptyExportNestedPath, `export const emptyExportNested = true`) const importedFiles = (await findAllImportedFiles(entryPath)).map((file) => normalizePath(file)) @@ -86,6 +96,10 @@ describe('findAllImportedFiles', () => { expect(importedFiles).toContain(normalizePath(valueNestedPath)) expect(importedFiles).toContain(normalizePath(mixedValuePath)) expect(importedFiles).toContain(normalizePath(mixedExportPath)) + expect(importedFiles).toContain(normalizePath(emptyImportPath)) + expect(importedFiles).toContain(normalizePath(emptyImportNestedPath)) + expect(importedFiles).toContain(normalizePath(emptyExportPath)) + expect(importedFiles).toContain(normalizePath(emptyExportNestedPath)) expect(importedFiles).not.toContain(normalizePath(typePath)) expect(importedFiles).not.toContain(normalizePath(typeNestedPath)) expect(importedFiles).not.toContain(normalizePath(exportedTypePath)) @@ -180,6 +194,57 @@ describe('findAllImportedFiles', () => { }) }) + test('uses explicit tsconfig include inherited from an extended config', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const extensionDir = joinPath(tmpDir, 'extensions', 'extension') + const srcDir = joinPath(extensionDir, 'src') + const excludedDir = joinPath(extensionDir, 'excluded') + + await mkdir(extensionDir) + await mkdir(srcDir) + await mkdir(excludedDir) + + const entryPath = joinPath(srcDir, 'index.ts') + const includedPath = joinPath(srcDir, 'included.ts') + const excludedPath = joinPath(excludedDir, 'excluded.ts') + + await writeFile( + joinPath(tmpDir, 'tsconfig.base.json'), + JSON.stringify({ + include: ['extensions/extension/src/**/*'], + }), + ) + await writeFile( + joinPath(extensionDir, 'tsconfig.json'), + JSON.stringify({ + extends: '../../tsconfig.base.json', + }), + ) + await writeFile( + entryPath, + ` + import './included.ts' + import '../excluded/excluded.ts' + `, + ) + await writeFile(includedPath, `export const included = true`) + await writeFile(excludedPath, `export const excluded = true`) + + const allowedFiles = await findExplicitTsConfigFiles(entryPath, extensionDir) + const importedFiles = ( + await findAllImportedFiles(entryPath, { + boundaryDirectory: extensionDir, + allowedFiles, + alwaysAllowedFiles: new Set([entryPath]), + }) + ).map((file) => normalizePath(file)) + + expect(allowedFiles).toContain(normalizePath(entryPath)) + expect(importedFiles).toContain(normalizePath(includedPath)) + expect(importedFiles).not.toContain(normalizePath(excludedPath)) + }) + }) + test('does not follow declaration files', async () => { await inTemporaryDirectory(async (tmpDir) => { const entryPath = joinPath(tmpDir, 'index.ts') diff --git a/packages/app/src/cli/models/extensions/specifications/type-generation.ts b/packages/app/src/cli/models/extensions/specifications/type-generation.ts index 56de6d60e65..59976b52f72 100644 --- a/packages/app/src/cli/models/extensions/specifications/type-generation.ts +++ b/packages/app/src/cli/models/extensions/specifications/type-generation.ts @@ -46,27 +46,44 @@ export function parseApiVersion(apiVersion: string): {year: number; month: numbe return {year: parseInt(year, 10), month: parseInt(month, 10)} } -async function loadTsConfig(startPath: string): Promise<{ +interface LoadedTsConfig { compilerOptions: ts.CompilerOptions configPath: string | undefined fileNames?: string[] hasExplicitFiles: boolean -}> { +} + +export type TsConfigCache = Map + +async function loadTsConfig(startPath: string, cache?: TsConfigCache): Promise { const ts = await loadTypeScript() const configPath = ts.findConfigFile(startPath, ts.sys.fileExists.bind(ts.sys), 'tsconfig.json') if (!configPath) { return {compilerOptions: {}, configPath: undefined, hasExplicitFiles: false} } + const resolvedConfigPath = resolvePath(configPath) + const cachedConfig = cache?.get(resolvedConfigPath) + if (cachedConfig) { + return cachedConfig + } + const configFile = ts.readConfigFile(configPath, ts.sys.readFile.bind(ts.sys)) if (configFile.error) { - return {compilerOptions: {}, configPath, hasExplicitFiles: false} + return {compilerOptions: {}, configPath: resolvedConfigPath, hasExplicitFiles: false} } const parsedConfig = ts.parseJsonConfigFileContent(configFile.config, ts.sys, dirname(configPath)) - const hasExplicitFiles = Boolean(configFile.config.files ?? configFile.config.include) + const hasExplicitFiles = Boolean(parsedConfig.raw?.files ?? parsedConfig.raw?.include) + const loadedConfig = { + compilerOptions: parsedConfig.options, + configPath: resolvedConfigPath, + fileNames: parsedConfig.fileNames, + hasExplicitFiles, + } - return {compilerOptions: parsedConfig.options, configPath, fileNames: parsedConfig.fileNames, hasExplicitFiles} + cache?.set(resolvedConfigPath, loadedConfig) + return loadedConfig } async function fallbackResolve(importPath: string, baseDir: string): Promise { @@ -104,10 +121,12 @@ interface FindAllImportedFilesOptions { allowedFiles?: Set alwaysAllowedFiles?: Set importCache?: Map + tsConfigCache?: TsConfigCache } interface ParseAndResolveImportsOptions { importCache?: Map + tsConfigCache?: TsConfigCache } function isWithinBoundary(filePath: string, boundaryDirectory?: string): boolean { @@ -147,7 +166,7 @@ async function parseAndResolveImports( const resolvedPaths: string[] = [] // Load TypeScript configuration once - const {compilerOptions} = await loadTsConfig(filePath) + const {compilerOptions} = await loadTsConfig(filePath, options.tsConfigCache) // Determine script kind based on file extension let scriptKind = ts.ScriptKind.JSX @@ -172,6 +191,7 @@ async function parseAndResolveImports( !node.importClause?.name && node.importClause?.namedBindings && ts.isNamedImports(node.importClause.namedBindings) && + node.importClause.namedBindings.elements.length > 0 && node.importClause.namedBindings.elements.every((element) => element.isTypeOnly) ) { return @@ -191,6 +211,7 @@ async function parseAndResolveImports( if ( node.exportClause && ts.isNamedExports(node.exportClause) && + node.exportClause.elements.length > 0 && node.exportClause.elements.every((element) => element.isTypeOnly) ) { return @@ -268,8 +289,9 @@ export async function findAllImportedFiles( export async function findExplicitTsConfigFiles( fromFile: string, extensionDirectory: string, + options: {tsConfigCache?: TsConfigCache} = {}, ): Promise | undefined> { - const {configPath, fileNames, hasExplicitFiles} = await loadTsConfig(fromFile) + const {configPath, fileNames, hasExplicitFiles} = await loadTsConfig(fromFile, options.tsConfigCache) if (!configPath || !hasExplicitFiles) return if (!isWithinBoundary(configPath, extensionDirectory)) return diff --git a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts index d6e0059b0f1..0312bc80480 100644 --- a/packages/app/src/cli/models/extensions/specifications/ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/ui_extension.ts @@ -8,6 +8,7 @@ import { getGeneratedTypesHelperImportPath, IntentSchemaFileSchema, parseApiVersion, + type TsConfigCache, ToolsFileSchema, } from './type-generation.js' import {Asset, AssetIdentifier, BuildAsset, ExtensionFeature, createExtensionSpecification} from '../specification.js' @@ -19,7 +20,7 @@ import {formatContent} from '../../../utilities/file-formatter.js' import {uniq} from '@shopify/cli-kit/common/array' import {err, ok, Result} from '@shopify/cli-kit/node/result' import {fileExists, readFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' +import {joinPath, resolvePath} from '@shopify/cli-kit/node/path' import {outputContent, outputToken, outputWarn} from '@shopify/cli-kit/node/output' import {zod} from '@shopify/cli-kit/node/schema' import {AbortError} from '@shopify/cli-kit/node/error' @@ -212,6 +213,7 @@ const uiExtensionSpec = createExtensionSpecification({ const fileToToolsMap = new Map() const fileToIntentsMap = new Map>() const importCache = new Map() + const tsConfigCache: TsConfigCache = new Map() // First pass: collect all entry point files and their targets for await (const extensionPoint of configuration.extension_points) { @@ -256,12 +258,13 @@ const uiExtensionSpec = createExtensionSpecification({ if (!exists) continue // Find all imported files recursively - const allowedFiles = await findExplicitTsConfigFiles(fullPath, extension.directory) + const allowedFiles = await findExplicitTsConfigFiles(fullPath, extension.directory, {tsConfigCache}) const importedFiles = await findAllImportedFiles(fullPath, { boundaryDirectory: extension.directory, allowedFiles, - alwaysAllowedFiles: new Set([fullPath]), + alwaysAllowedFiles: new Set([resolvePath(fullPath)]), importCache, + tsConfigCache, }) // Associate imported files with this extension point's target @@ -279,12 +282,15 @@ const uiExtensionSpec = createExtensionSpecification({ ) const shouldRenderExists = await fileExists(shouldRenderPath) if (shouldRenderExists) { - const shouldRenderAllowedFiles = await findExplicitTsConfigFiles(shouldRenderPath, extension.directory) + const shouldRenderAllowedFiles = await findExplicitTsConfigFiles(shouldRenderPath, extension.directory, { + tsConfigCache, + }) const shouldRenderImports = await findAllImportedFiles(shouldRenderPath, { boundaryDirectory: extension.directory, allowedFiles: shouldRenderAllowedFiles, - alwaysAllowedFiles: new Set([shouldRenderPath]), + alwaysAllowedFiles: new Set([resolvePath(shouldRenderPath)]), importCache, + tsConfigCache, }) for (const importedFile of shouldRenderImports) { const currentTargets = fileToTargetsMap.get(importedFile) ?? []