From 16de77afb91ea941bbf7702ce03596b80dca2c8f Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 8 Jun 2026 11:48:15 +0200 Subject: [PATCH 1/3] 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 c6137c646426d7af4ea5e157e20b5f4e741e1fa7 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 9 Jun 2026 17:52:39 +0200 Subject: [PATCH 2/3] 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 8852a802bcdd005fc723c1ba31d996e19cc62706 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 10 Jun 2026 11:24:17 +0200 Subject: [PATCH 3/3] 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) ?? []