diff --git a/.changeset/autocomplete-instant-local-search.md b/.changeset/autocomplete-instant-local-search.md deleted file mode 100644 index d515d7b298f..00000000000 --- a/.changeset/autocomplete-instant-local-search.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@shopify/cli-kit': patch ---- - -Make the autocomplete prompt feel instant when filtering against in-memory choices. diff --git a/.changeset/guard-bundle-upload-size-and-paths.md b/.changeset/guard-bundle-upload-size-and-paths.md deleted file mode 100644 index 4508c32ff75..00000000000 --- a/.changeset/guard-bundle-upload-size-and-paths.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@shopify/app': patch ---- - -Guard app bundle uploads against oversized bundles and asset paths resolving outside the app directory diff --git a/packages/app/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts b/packages/app/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts index 52e20a2dd47..fd627c21b24 100644 --- a/packages/app/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts +++ b/packages/app/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts @@ -22,6 +22,8 @@ export type Scalars = { ActionAuditID: { input: any; output: any; } /** The ID for a Address. */ AddressID: { input: any; output: any; } + /** The ID for a Attestation. */ + AttestationID: { input: any; output: any; } /** The ID for a BulkDataOperation. */ BulkDataOperationID: { input: any; output: any; } /** The ID for a BusinessUser. */ diff --git a/packages/app/src/cli/commands/app/generate/extension.ts b/packages/app/src/cli/commands/app/generate/extension.ts index b0ea709fb9a..3d16eab7984 100644 --- a/packages/app/src/cli/commands/app/generate/extension.ts +++ b/packages/app/src/cli/commands/app/generate/extension.ts @@ -47,6 +47,10 @@ export default class AppGenerateExtension extends AppLinkedCommand { }), } + public static analyticsNameOverride(): string | undefined { + return 'app scaffold extension' + } + public async run(): Promise { const {flags} = await this.parse(AppGenerateExtension) diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts index e493f046643..e9decd6b02d 100644 --- a/packages/app/src/cli/services/build/steps/include-assets-step.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets-step.test.ts @@ -25,7 +25,7 @@ describe('executeIncludeAssetsStep', () => { options: { stdout: mockStdout, stderr: mockStderr, - app: {directory: '/test'} as any, + app: {} as any, environment: 'production', }, stepResults: new Map(), @@ -552,11 +552,6 @@ describe('executeIncludeAssetsStep', () => { }) describe('pattern entries', () => { - beforeEach(() => { - // copyByPattern now short-circuits if sourceDir doesn't exist; default true here. - vi.mocked(fs.fileExists).mockResolvedValue(true) - }) - test('copies files matching include patterns', async () => { // Given vi.mocked(fs.glob).mockResolvedValue(['/test/extension/public/logo.png', '/test/extension/public/style.css']) @@ -948,13 +943,11 @@ describe('executeIncludeAssetsStep', () => { }) test('writes manifest.json with files array when generatesAssetsManifest is true and only pattern inclusions exist', async () => { - // Given — pattern entries contribute output paths to the manifest "files" array. - // sourceDir must exist for copyByPattern's pre-glob fileExists check to pass; - // everything else can read false (the parent beforeEach default). + // Given — pattern entries contribute output paths to the manifest "files" array vi.mocked(fs.glob).mockResolvedValue(['/test/extension/public/logo.png']) vi.mocked(fs.copyFile).mockResolvedValue() vi.mocked(fs.mkdir).mockResolvedValue() - vi.mocked(fs.fileExists).mockImplementation(async (path) => String(path) === '/test/extension/public') + vi.mocked(fs.fileExists).mockResolvedValue(false) vi.mocked(fs.writeFile).mockResolvedValue() const step: LifecycleStep = { diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.ts b/packages/app/src/cli/services/build/steps/include-assets-step.ts index dc7bdba614f..1aba68d5d5d 100644 --- a/packages/app/src/cli/services/build/steps/include-assets-step.ts +++ b/packages/app/src/cli/services/build/steps/include-assets-step.ts @@ -125,7 +125,6 @@ export async function executeIncludeAssetsStep( const config = IncludeAssetsConfigSchema.parse(step.config) const {extension, options} = context const outputDir = resolveOutputDir(extension.outputPath) - const appDirectory = options.app.directory const aggregatedPathMap = new Map() // Track basenames written across all configKey entries in this build. In @@ -148,7 +147,6 @@ export async function executeIncludeAssetsStep( baseDir: extension.directory, outputDir, context, - appDirectory, destination: sanitizedDest, usedBasenames, preserveFilePaths: entry.preserveFilePaths, @@ -174,8 +172,6 @@ export async function executeIncludeAssetsStep( outputDir: destinationDir, patterns: entry.include, ignore: entry.ignore ?? [], - appDirectory, - sourceDirConfigValue: entry.baseDir ?? '.', }, options, ) @@ -193,7 +189,6 @@ export async function executeIncludeAssetsStep( destination: sanitizedDest, baseDir: extension.directory, outputDir, - appDirectory, }, options, ) diff --git a/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts b/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts deleted file mode 100644 index c6f2634c5c8..00000000000 --- a/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts +++ /dev/null @@ -1,101 +0,0 @@ -import {assertPathWithinAppDir} from './assert-path-within-app.js' -import {AbortError} from '@shopify/cli-kit/node/error' -import {inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' -import {describe, expect, test} from 'vitest' -import {symlink} from 'fs/promises' - -describe('assertPathWithinAppDir', () => { - test('allows a path inside the app directory', async () => { - await inTemporaryDirectory(async (appDir) => { - const inside = joinPath(appDir, 'extensions', 'ext-a') - await mkdir(inside) - await writeFile(joinPath(inside, 'icon.png'), 'x') - await expect( - assertPathWithinAppDir(joinPath(inside, 'icon.png'), appDir, 'extensions/ext-a/icon.png'), - ).resolves.toBeUndefined() - }) - }) - - test('allows the app directory itself', async () => { - await inTemporaryDirectory(async (appDir) => { - await expect(assertPathWithinAppDir(appDir, appDir, '.')).resolves.toBeUndefined() - }) - }) - - test('rejects a path that resolves outside the app directory via ..', async () => { - await inTemporaryDirectory(async (parent) => { - const appDir = joinPath(parent, 'app') - await mkdir(appDir) - const outside = joinPath(parent, 'outside.json') - await writeFile(outside, '{}') - await expect(assertPathWithinAppDir(outside, appDir, '../outside.json')).rejects.toThrow(AbortError) - await expect(assertPathWithinAppDir(outside, appDir, '../outside.json')).rejects.toThrow( - /resolves outside the app directory/, - ) - }) - }) - - test('rejects a symlink whose target is outside the app directory', async () => { - await inTemporaryDirectory(async (parent) => { - const appDir = joinPath(parent, 'app') - await mkdir(appDir) - const outsideDir = joinPath(parent, 'home', 'big-folder') - await mkdir(outsideDir) - await writeFile(joinPath(outsideDir, 'huge.bin'), 'x') - - // Inside the app dir, but the symlink points outside. - const symlinkInApp = joinPath(appDir, 'assets') - await symlink(outsideDir, symlinkInApp) - - await expect(assertPathWithinAppDir(symlinkInApp, appDir, 'assets')).rejects.toThrow(AbortError) - }) - }) - - test('allows an in-tree symlink (e.g. pnpm-style links staying inside the app)', async () => { - await inTemporaryDirectory(async (appDir) => { - const realTarget = joinPath(appDir, 'shared') - await mkdir(realTarget) - const linkPath = joinPath(appDir, 'extensions', 'ext-a-assets') - await mkdir(joinPath(appDir, 'extensions')) - await symlink(realTarget, linkPath) - - await expect(assertPathWithinAppDir(linkPath, appDir, 'extensions/ext-a-assets')).resolves.toBeUndefined() - }) - }) - - test('does not false-positive on macOS-style symlinked temp dirs (both sides realpath’d)', async () => { - // inTemporaryDirectory on macOS returns a /var/folders/... path whose - // realpath is /private/var/folders/.... If only the source were realpath’d - // the check would treat the temp dir as outside itself. - await inTemporaryDirectory(async (appDir) => { - const inside = joinPath(appDir, 'src') - await mkdir(inside) - await writeFile(joinPath(inside, 'schema.json'), '{}') - await expect( - assertPathWithinAppDir(joinPath(inside, 'schema.json'), appDir, 'src/schema.json'), - ).resolves.toBeUndefined() - }) - }) - - test('allows a sibling whose name starts with two dots (e.g. ..cache)', async () => { - await inTemporaryDirectory(async (appDir) => { - const dotdotDir = joinPath(appDir, '..cache') - await mkdir(dotdotDir) - await writeFile(joinPath(dotdotDir, 'file.txt'), 'x') - await expect( - assertPathWithinAppDir(joinPath(dotdotDir, 'file.txt'), appDir, '..cache/file.txt'), - ).resolves.toBeUndefined() - }) - }) - - test('includes the original config value in the error for debuggability', async () => { - await inTemporaryDirectory(async (parent) => { - const appDir = joinPath(parent, 'app') - await mkdir(appDir) - const outside = joinPath(parent, 'leak') - await writeFile(outside, '') - await expect(assertPathWithinAppDir(outside, appDir, '~/anywhere')).rejects.toThrow(/Asset path '~\/anywhere'/) - }) - }) -}) diff --git a/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts b/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts deleted file mode 100644 index f875e599ff3..00000000000 --- a/packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts +++ /dev/null @@ -1,37 +0,0 @@ -import {relativePath, isAbsolutePath} from '@shopify/cli-kit/node/path' -import {fileRealPath} from '@shopify/cli-kit/node/fs' -import {AbortError} from '@shopify/cli-kit/node/error' - -/** - * Throws if `resolvedPath` is not inside `appDirectory` after symlink resolution. - * - * Guards against accidental misuse where an extension config points an asset - * source outside the app folder — either via `..`/absolute paths or by - * symlinking an in-app directory to somewhere else (e.g. a home directory). - * - * Both sides are realpath'd before comparison so macOS temp paths - * (`/var/folders` → `/private/var/folders`) and pnpm-style in-tree symlinks - * don't trip a false positive. - * - * `configValue` is the raw value from configuration used in the error message - * so the user can locate the offending entry. Caller must ensure `resolvedPath` - * exists on disk — `fileRealPath` throws on missing paths. - */ -export async function assertPathWithinAppDir( - resolvedPath: string, - appDirectory: string, - configValue: string, -): Promise { - const [realSource, realAppDir] = await Promise.all([fileRealPath(resolvedPath), fileRealPath(appDirectory)]) - const relative = relativePath(realAppDir, realSource) - // Check `..` as a path segment, not just a prefix. A sibling-style name like - // `..cache` is a valid in-app entry whose relative path begins with `..` but - // does not escape. - const firstSegment = relative.split(/[/\\]/, 1)[0] - if (firstSegment === '..' || isAbsolutePath(relative)) { - throw new AbortError( - `Asset path '${configValue}' resolves outside the app directory.`, - `Asset sources must be inside the app folder. Resolved to: ${realSource}`, - ) - } -} diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts index 6a3e4b81df7..ea4cf855116 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts @@ -9,7 +9,6 @@ describe('copyByPattern', () => { beforeEach(() => { mockStdout = {write: vi.fn()} - vi.mocked(fs.fileExists).mockResolvedValue(true) }) test('copies matched files preserving relative paths', async () => { @@ -25,8 +24,6 @@ describe('copyByPattern', () => { outputDir: '/out', patterns: ['**/*.ts', '**/*.tsx'], ignore: [], - appDirectory: '/src', - sourceDirConfigValue: '.', }, {stdout: mockStdout}, ) @@ -50,8 +47,6 @@ describe('copyByPattern', () => { outputDir: '/out', patterns: ['**/*.png'], ignore: [], - appDirectory: '/src', - sourceDirConfigValue: '.', }, {stdout: mockStdout}, ) @@ -77,8 +72,6 @@ describe('copyByPattern', () => { outputDir: '/out/sub', patterns: ['**/*'], ignore: [], - appDirectory: '/out', - sourceDirConfigValue: 'sub', }, {stdout: mockStdout}, ) @@ -103,8 +96,6 @@ describe('copyByPattern', () => { outputDir: '/out', patterns: ['*.png'], ignore: [], - appDirectory: '/out', - sourceDirConfigValue: '.', }, {stdout: mockStdout}, ) @@ -129,8 +120,6 @@ describe('copyByPattern', () => { outputDir: '/out/dist', patterns: ['*.js'], ignore: [], - appDirectory: '/src', - sourceDirConfigValue: '.', }, {stdout: mockStdout}, ) @@ -150,8 +139,6 @@ describe('copyByPattern', () => { outputDir: '/out', patterns: ['**/*'], ignore: ['**/*.test.ts', 'node_modules/**'], - appDirectory: '/src', - sourceDirConfigValue: '.', }, {stdout: mockStdout}, ) diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts index 0493973a56f..c1716d7d2da 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts @@ -1,6 +1,5 @@ -import {assertPathWithinAppDir} from './assert-path-within-app.js' import {joinPath, dirname, relativePath} from '@shopify/cli-kit/node/path' -import {glob, copyFile, mkdir, fileExists} from '@shopify/cli-kit/node/fs' +import {glob, copyFile, mkdir} from '@shopify/cli-kit/node/fs' /** * Pattern strategy: glob-based file selection. @@ -11,21 +10,10 @@ export async function copyByPattern( outputDir: string patterns: string[] ignore: string[] - appDirectory: string - sourceDirConfigValue: string }, options: {stdout: NodeJS.WritableStream}, ): Promise<{filesCopied: number; outputPaths: string[]}> { - const {sourceDir, outputDir, patterns, ignore, appDirectory, sourceDirConfigValue} = config - - // Validate the boundary up front, before touching the filesystem. Realpath - // would throw on a missing sourceDir, so preserve the existing "missing dir - // = no files" behavior by short-circuiting first. - if (!(await fileExists(sourceDir))) { - return {filesCopied: 0, outputPaths: []} - } - await assertPathWithinAppDir(sourceDir, appDirectory, sourceDirConfigValue) - + const {sourceDir, outputDir, patterns, ignore} = config const files = await glob(patterns, { absolute: true, cwd: sourceDir, diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts index 9e6cddf87b9..70a12187d21 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts @@ -28,13 +28,7 @@ describe('copyConfigKeyEntry', () => { await mkdir(outDir) const context = makeContext({static_root: 'public'}, mockStdout) - const result = await copyConfigKeyEntry({ - key: 'static_root', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) + const result = await copyConfigKeyEntry({key: 'static_root', baseDir: tmpDir, outputDir: outDir, context}) expect(result.filesCopied).toBe(2) await expect(fileExists(joinPath(outDir, 'index.html'))).resolves.toBe(true) @@ -54,13 +48,7 @@ describe('copyConfigKeyEntry', () => { await mkdir(outDir) const context = makeContext({schema_path: 'src/schema.json'}, mockStdout) - const result = await copyConfigKeyEntry({ - key: 'schema_path', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) + const result = await copyConfigKeyEntry({key: 'schema_path', baseDir: tmpDir, outputDir: outDir, context}) expect(result.filesCopied).toBe(1) await expect(fileExists(joinPath(outDir, 'schema.json'))).resolves.toBe(true) @@ -84,13 +72,7 @@ describe('copyConfigKeyEntry', () => { await mkdir(outDir) const context = makeContext({files: ['a/schema.json', 'b/schema.json']}, mockStdout) - const result = await copyConfigKeyEntry({ - key: 'files', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) + const result = await copyConfigKeyEntry({key: 'files', baseDir: tmpDir, outputDir: outDir, context}) expect(result.filesCopied).toBe(2) // Both have basename schema.json — second one gets renamed @@ -105,13 +87,7 @@ describe('copyConfigKeyEntry', () => { await mkdir(outDir) const context = makeContext({}, mockStdout) - const result = await copyConfigKeyEntry({ - key: 'static_root', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) + const result = await copyConfigKeyEntry({key: 'static_root', baseDir: tmpDir, outputDir: outDir, context}) expect(result.filesCopied).toBe(0) expect(result.pathMap.size).toBe(0) @@ -125,7 +101,7 @@ describe('copyConfigKeyEntry', () => { const context = makeContext({assets_dir: 'nonexistent'}, mockStdout) await expect( - copyConfigKeyEntry({key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context, appDirectory: tmpDir}), + copyConfigKeyEntry({key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context}), ).rejects.toThrow(`Couldn't find`) }) }) @@ -145,13 +121,7 @@ describe('copyConfigKeyEntry', () => { await mkdir(outDir) const context = makeContext({roots: ['public', 'assets']}, mockStdout) - const result = await copyConfigKeyEntry({ - key: 'roots', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) + const result = await copyConfigKeyEntry({key: 'roots', baseDir: tmpDir, outputDir: outDir, context}) // Promise.all runs copies sequentially; glob on the shared outDir may see files // from the other copy, so the total count is at least 3 (one per real file). @@ -177,7 +147,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, destination: 'static/icons', }) @@ -205,7 +174,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, }) expect(result.filesCopied).toBe(3) @@ -232,7 +200,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, }) expect(result.filesCopied).toBe(0) @@ -260,7 +227,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context: contextA, - appDirectory: tmpDir, usedBasenames, preserveFilePaths: true, }) @@ -271,7 +237,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context: contextB, - appDirectory: tmpDir, usedBasenames, preserveFilePaths: true, }) @@ -298,7 +263,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, preserveFilePaths: true, }) await expect(promise).rejects.toThrow(AbortError) @@ -321,7 +285,6 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, preserveFilePaths: true, }) @@ -347,64 +310,10 @@ describe('copyConfigKeyEntry', () => { baseDir: tmpDir, outputDir: outDir, context, - appDirectory: tmpDir, }) expect(result.filesCopied).toBe(1) await expect(fileExists(joinPath(outDir, 'tools.json'))).resolves.toBe(true) }) }) - - describe('value guard', () => { - test('throws when value is an empty string', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const outDir = joinPath(tmpDir, 'out') - await mkdir(outDir) - const context = makeContext({assets: ''}) - const promise = copyConfigKeyEntry({ - key: 'assets', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) - await expect(promise).rejects.toThrow(AbortError) - await expect(promise).rejects.toThrow(`'assets' can't be empty.`) - }) - }) - - test('throws when value is whitespace-only', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const outDir = joinPath(tmpDir, 'out') - await mkdir(outDir) - const context = makeContext({assets: ' '}) - const promise = copyConfigKeyEntry({ - key: 'assets', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) - await expect(promise).rejects.toThrow(AbortError) - await expect(promise).rejects.toThrow(`'assets' can't be empty.`) - }) - }) - - test('throws with the full configKey when the key is nested', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const outDir = joinPath(tmpDir, 'out') - await mkdir(outDir) - const context = makeContext({extension_points: [{assets: ''}]}) - const promise = copyConfigKeyEntry({ - key: 'extension_points[].assets', - baseDir: tmpDir, - outputDir: outDir, - context, - appDirectory: tmpDir, - }) - await expect(promise).rejects.toThrow(AbortError) - await expect(promise).rejects.toThrow(`'extension_points[].assets' can't be empty.`) - }) - }) - }) }) diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts index c952cd8abf1..edb9b9e6b38 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts @@ -1,4 +1,3 @@ -import {assertPathWithinAppDir} from './assert-path-within-app.js' import {joinPath, basename, relativePath, extname} from '@shopify/cli-kit/node/path' import {glob, copyFile, copyDirectoryContents, fileExists, mkdir, isDirectory} from '@shopify/cli-kit/node/fs' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' @@ -25,7 +24,6 @@ export async function copyConfigKeyEntry(config: { baseDir: string outputDir: string context: BuildContext - appDirectory: string destination?: string usedBasenames?: Set preserveFilePaths?: boolean @@ -35,7 +33,6 @@ export async function copyConfigKeyEntry(config: { baseDir, outputDir, context, - appDirectory, destination, usedBasenames = new Set(), preserveFilePaths = false, @@ -51,12 +48,6 @@ export async function copyConfigKeyEntry(config: { paths = [] } - for (const sourcePath of paths) { - if (sourcePath.trim() === '') { - throw new AbortError(`'${key}' can't be empty.`) - } - } - if (paths.length === 0) { outputDebug(`No value for configKey '${key}', skipping\n`, stdout) return {filesCopied: 0, pathMap: new Map()} @@ -82,7 +73,6 @@ export async function copyConfigKeyEntry(config: { .value, ) } - await assertPathWithinAppDir(fullPath, appDirectory, sourcePath) const sourceIsDir = await isDirectory(fullPath) diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts index 5c1436fc2d2..1a98fdb70a8 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts @@ -23,7 +23,6 @@ describe('copySourceEntry', () => { destination: undefined, baseDir: '/ext', outputDir: '/out', - appDirectory: '/ext', }, {stdout: mockStdout}, ), @@ -44,7 +43,6 @@ describe('copySourceEntry', () => { destination: 'assets/icon.png', baseDir: '/ext', outputDir: '/out', - appDirectory: '/ext', }, {stdout: mockStdout}, ) @@ -65,7 +63,7 @@ describe('copySourceEntry', () => { // When const result = await copySourceEntry( - {source: 'dist', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, + {source: 'dist', destination: undefined, baseDir: '/ext', outputDir: '/out'}, {stdout: mockStdout}, ) @@ -85,7 +83,7 @@ describe('copySourceEntry', () => { // When const result = await copySourceEntry( - {source: 'README.md', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, + {source: 'README.md', destination: undefined, baseDir: '/ext', outputDir: '/out'}, {stdout: mockStdout}, ) @@ -105,7 +103,7 @@ describe('copySourceEntry', () => { // When const result = await copySourceEntry( - {source: 'dist', destination: 'vendor/dist', baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, + {source: 'dist', destination: 'vendor/dist', baseDir: '/ext', outputDir: '/out'}, {stdout: mockStdout}, ) @@ -126,7 +124,7 @@ describe('copySourceEntry', () => { // When const result = await copySourceEntry( - {source: 'theme', destination: undefined, baseDir: '/ext', outputDir: '/out', appDirectory: '/ext'}, + {source: 'theme', destination: undefined, baseDir: '/ext', outputDir: '/out'}, {stdout: mockStdout}, ) @@ -149,7 +147,6 @@ describe('copySourceEntry', () => { destination: 'assets/icons/icon.png', baseDir: '/ext', outputDir: '/out', - appDirectory: '/ext', }, {stdout: mockStdout}, ) diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts index 6aa75a9823b..f573338b74b 100644 --- a/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts +++ b/packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts @@ -1,4 +1,3 @@ -import {assertPathWithinAppDir} from './assert-path-within-app.js' import {joinPath, dirname, basename, relativePath} from '@shopify/cli-kit/node/path' import {glob, copyFile, copyDirectoryContents, fileExists, mkdir, isDirectory} from '@shopify/cli-kit/node/fs' @@ -14,16 +13,14 @@ export async function copySourceEntry( destination: string | undefined baseDir: string outputDir: string - appDirectory: string }, options: {stdout: NodeJS.WritableStream}, ): Promise<{filesCopied: number; outputPaths: string[]}> { - const {source, destination, baseDir, outputDir, appDirectory} = config + const {source, destination, baseDir, outputDir} = config const sourcePath = joinPath(baseDir, source) if (!(await fileExists(sourcePath))) { throw new Error(`Source does not exist: ${sourcePath}`) } - await assertPathWithinAppDir(sourcePath, appDirectory, source) const sourceIsDir = await isDirectory(sourcePath) diff --git a/packages/app/src/cli/services/bundle.test.ts b/packages/app/src/cli/services/bundle.test.ts index 35fc7679335..0adf8cbe87a 100644 --- a/packages/app/src/cli/services/bundle.test.ts +++ b/packages/app/src/cli/services/bundle.test.ts @@ -1,11 +1,9 @@ -import {writeManifestToBundle, compressBundle, uploadToGCS, BUNDLE_EXCLUSION_PATTERNS} from './bundle.js' +import {writeManifestToBundle, compressBundle, BUNDLE_EXCLUSION_PATTERNS} from './bundle.js' import {AppInterface} from '../models/app/app.js' import {describe, test, expect, vi} from 'vitest' import {joinPath} from '@shopify/cli-kit/node/path' -import {inTemporaryDirectory, mkdir, writeFile, readFile, fileSize} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, mkdir, writeFile, readFile} from '@shopify/cli-kit/node/fs' import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver' -import {AbortError} from '@shopify/cli-kit/node/error' -import {fetch} from '@shopify/cli-kit/node/http' vi.mock('@shopify/cli-kit/node/archiver', () => { return { @@ -14,16 +12,6 @@ vi.mock('@shopify/cli-kit/node/archiver', () => { } }) -vi.mock('@shopify/cli-kit/node/http', async (importActual) => { - const actual: any = await importActual() - return {...actual, fetch: vi.fn()} -}) - -vi.mock('@shopify/cli-kit/node/fs', async (importActual) => { - const actual: any = await importActual() - return {...actual, fileSize: vi.fn(actual.fileSize)} -}) - describe('writeManifestToBundle', () => { test('writes manifest.json to the specified directory', async () => { await inTemporaryDirectory(async (tmpDir) => { @@ -204,42 +192,3 @@ describe('compressBundle', () => { ) }) }) - -describe('uploadToGCS', () => { - test('uploads the bundle when it is under the size limit', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const bundlePath = joinPath(tmpDir, 'bundle.zip') - await writeFile(bundlePath, 'small contents') - vi.mocked(fetch).mockResolvedValue({} as never) - - // When - await uploadToGCS('https://signed.example/upload', bundlePath) - - // Then - expect(fetch).toHaveBeenCalledWith( - 'https://signed.example/upload', - expect.objectContaining({method: 'put'}), - 'slow-request', - ) - }) - }) - - test('aborts with a helpful error when the bundle exceeds the 100 MB upload limit', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given — mock the reported size so CI doesn't have to allocate 101 MB on disk. - const bundlePath = joinPath(tmpDir, 'huge.zip') - await writeFile(bundlePath, 'placeholder') - const oneHundredOneMb = 101 * 1024 * 1024 - vi.mocked(fileSize).mockResolvedValueOnce(oneHundredOneMb).mockResolvedValueOnce(oneHundredOneMb) - vi.mocked(fetch).mockResolvedValue({} as never) - - // When / Then - await expect(uploadToGCS('https://signed.example/upload', bundlePath)).rejects.toThrow(AbortError) - await expect(uploadToGCS('https://signed.example/upload', bundlePath)).rejects.toThrow( - /exceeds the 100 MB upload limit/, - ) - expect(fetch).not.toHaveBeenCalled() - }) - }) -}) diff --git a/packages/app/src/cli/services/bundle.ts b/packages/app/src/cli/services/bundle.ts index bd853c31753..f849da2a77d 100644 --- a/packages/app/src/cli/services/bundle.ts +++ b/packages/app/src/cli/services/bundle.ts @@ -5,14 +5,10 @@ import {MinimalAppIdentifiers} from '../models/organization.js' import {joinPath} from '@shopify/cli-kit/node/path' import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver' import {formData, fetch} from '@shopify/cli-kit/node/http' -import {fileSize, readFileSync} from '@shopify/cli-kit/node/fs' +import {readFileSync} from '@shopify/cli-kit/node/fs' import {AbortError} from '@shopify/cli-kit/node/error' import {writeFile} from 'fs/promises' -const MEGABYTE = 1024 * 1024 -const MAX_BUNDLE_SIZE_MB = 100 -const MAX_BUNDLE_SIZE_BYTES = MAX_BUNDLE_SIZE_MB * MEGABYTE - export async function writeManifestToBundle(appManifest: AppManifest, bundlePath: string) { const manifestPath = joinPath(bundlePath, 'manifest.json') await writeFile(manifestPath, JSON.stringify(appManifest, null, 2)) @@ -35,15 +31,6 @@ export async function compressBundle(inputDirectory: string, outputPath: string, * @param filePath - The path to the file */ export async function uploadToGCS(signedURL: string, filePath: string) { - const size = await fileSize(filePath) - if (size > MAX_BUNDLE_SIZE_BYTES) { - // Round up so a size that barely exceeds the cap never displays as the cap. - const humanSize = `${(Math.ceil((size / MEGABYTE) * 100) / 100).toFixed(2)} MB` - throw new AbortError( - `Your app bundle exceeds the ${MAX_BUNDLE_SIZE_MB} MB upload limit (it is ${humanSize}).`, - `Check the asset paths in your extension configuration — a misconfigured source can pull in much more than intended. Exclude large files or directories from your bundle, then try again.`, - ) - } const form = formData() const buffer = readFileSync(filePath) form.append('my_upload', buffer) diff --git a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts index 206d63ed971..2e689898ed0 100644 --- a/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts +++ b/packages/app/src/cli/services/dev/extension/server/middlewares.test.ts @@ -405,7 +405,6 @@ describe('getExtensionAssetMiddleware()', () => { baseDir: extDir, outputDir, context: {extension, options: {stdout: {write: vi.fn()}}} as any, - appDirectory: tmpDir, }) const flattened = buildResult.pathMap.get('../tools.json') as string diff --git a/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx b/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx index 2808a12dbba..013035dfe7b 100644 --- a/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx +++ b/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx @@ -27,13 +27,6 @@ vi.mock('@shopify/cli-kit/node/system', async () => { vi.mock('../../../context.js') vi.mock('../../fetch.js') vi.mock('../../processes/dev-session.js') -vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => { - const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun') - return { - ...actual, - waitForPostRunHookAndExit: vi.fn(), - } -}) const developerPlatformClient = testDeveloperPlatformClient() diff --git a/packages/app/src/cli/services/dev/ui/components/Dev.tsx b/packages/app/src/cli/services/dev/ui/components/Dev.tsx index 32f7f163666..eb93e7f529a 100644 --- a/packages/app/src/cli/services/dev/ui/components/Dev.tsx +++ b/packages/app/src/cli/services/dev/ui/components/Dev.tsx @@ -10,7 +10,8 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink' import {handleCtrlC} from '@shopify/cli-kit/node/ui' import {openURL} from '@shopify/cli-kit/node/system' import figures from '@shopify/cli-kit/node/figures' -import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun' +import {isUnitTest} from '@shopify/cli-kit/node/context/local' +import {treeKill} from '@shopify/cli-kit/node/tree-kill' import {Writable} from 'stream' export interface DeveloperPreviewController { @@ -73,7 +74,12 @@ const Dev: FunctionComponent = ({ setIsShuttingDownMessage('Shutting down dev because of an error ...') } else { setIsShuttingDownMessage('Shutting down dev ...') - waitForPostRunHookAndExit() + setTimeout(() => { + if (isUnitTest()) return + treeKill(process.pid, 'SIGINT', false, () => { + process.exit(0) + }) + }, 2000) } clearInterval(pollingInterval.current) await developerPreview.disable() diff --git a/packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx b/packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx index bf7ecbf24b2..06c85d56802 100644 --- a/packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx +++ b/packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx @@ -24,13 +24,6 @@ vi.mock('@shopify/cli-kit/node/system', async () => { }) vi.mock('@shopify/cli-kit/node/context/local') vi.mock('@shopify/cli-kit/node/tree-kill') -vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => { - const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun') - return { - ...actual, - waitForPostRunHookAndExit: vi.fn(), - } -}) const mocks = vi.hoisted(() => { return { diff --git a/packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx b/packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx index 058d663dace..6f99d6ed7b4 100644 --- a/packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx +++ b/packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx @@ -17,7 +17,9 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink' import {handleCtrlC} from '@shopify/cli-kit/node/ui' import {openURL, terminalSupportsHyperlinks} from '@shopify/cli-kit/node/system' import figures from '@shopify/cli-kit/node/figures' -import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun' +import {isUnitTest} from '@shopify/cli-kit/node/context/local' +import {treeKill} from '@shopify/cli-kit/node/tree-kill' +import {postRunHookHasCompleted} from '@shopify/cli-kit/node/hooks/postrun' import {Writable} from 'stream' interface DevStatusShortcut extends TabShortcut { @@ -68,7 +70,18 @@ const DevSessionUI: FunctionComponent = ({ setIsShuttingDownMessage('Shutting down dev ...') await onAbort() } - waitForPostRunHookAndExit() + if (isUnitTest()) return + + // Wait for the post run hook to complete or timeout after 5 seconds. + let totalTime = 0 + setInterval(() => { + if (postRunHookHasCompleted() || totalTime > 5000) { + treeKill(process.pid, 'SIGINT', false, () => { + process.exit(0) + }) + } + totalTime += 100 + }, 100) }) const errorHandledProcesses = useMemo(() => { diff --git a/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.test.tsx b/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.test.tsx index b909d64b7e3..7c449a9d710 100644 --- a/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.test.tsx @@ -620,34 +620,6 @@ describe('AutocompletePrompt', async () => { `) }) - test('searchDebounceMs: 0 invokes search on every keystroke without throttling', async () => { - const search = vi.fn(async (term: string) => ({ - data: DATABASE.filter((item) => item.label.includes(term)), - })) - - const renderInstance = render( - {}} - search={search} - searchDebounceMs={0} - />, - ) - - await waitForInputsToBeReady() - await sendInputAndWaitForChange(renderInstance, 'f') - await sendInputAndWaitForChange(renderInstance, 'i') - await sendInputAndWaitForChange(renderInstance, 'r') - - // With the default 400ms throttle, three rapid keystrokes coalesce to ~2 calls - // (leading + trailing edge). With searchDebounceMs=0, each keystroke fires. - expect(search).toHaveBeenCalledTimes(3) - expect(search).toHaveBeenNthCalledWith(1, 'f') - expect(search).toHaveBeenNthCalledWith(2, 'fi') - expect(search).toHaveBeenNthCalledWith(3, 'fir') - }) - test('displays an error message if the search fails', async () => { const search = (_term: string) => { return Promise.reject(new Error('Something went wrong')) diff --git a/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx b/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx index 7214cec2b11..01f277dfe66 100644 --- a/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx +++ b/packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx @@ -28,17 +28,9 @@ export interface AutocompletePromptProps { abortSignal?: AbortSignal infoMessage?: InfoMessageProps['message'] groupOrder?: string[] - /** - * Throttle window in milliseconds applied to the search callback. Defaults to 400ms, - * which is appropriate for remote/paginated backends. In-memory consumers (where the - * search callback resolves synchronously) can pass 0 for instant filtering on every - * keystroke. - */ - searchDebounceMs?: number } const MIN_NUMBER_OF_ITEMS_FOR_SEARCH = 5 -const DEFAULT_SEARCH_DEBOUNCE_MS = 400 function AutocompletePrompt({ message, @@ -50,7 +42,6 @@ function AutocompletePrompt({ abortSignal, infoMessage, groupOrder, - searchDebounceMs = DEFAULT_SEARCH_DEBOUNCE_MS, }: React.PropsWithChildren>): ReactElement | null { const complete = useComplete() const [searchTerm, setSearchTerm] = useState('') @@ -130,10 +121,10 @@ function AutocompletePrompt({ clearTimeout(setLoadingWhenSlow.current) }) }, - searchDebounceMs, + 400, {leading: true, trailing: true}, ), - [paginatedSearch, setPromptState, searchDebounceMs], + [paginatedSearch, setPromptState], ) return ( diff --git a/packages/cli-kit/src/public/common/string.ts b/packages/cli-kit/src/public/common/string.ts index 0e3e295df4d..cc655042ace 100644 --- a/packages/cli-kit/src/public/common/string.ts +++ b/packages/cli-kit/src/public/common/string.ts @@ -251,16 +251,34 @@ export function tryParseInt(maybeInt: string | undefined): number | undefined { * @returns A string with the columns aligned. */ export function linesToColumns(lines: string[][]): string { - const widths: number[] = [] - for (let i = 0; lines[0] && i < lines[0].length; i++) { - const columnRows = lines.map((line) => line[i]!) - widths.push(Math.max(...columnRows.map((row) => unstyled(row).length))) - } + if (lines.length === 0) return '' + + const numColumns = lines[0]!.length + const widths: number[] = new Array(numColumns).fill(0) + + // Optimization: Calculate max widths for all columns while caching unstyled lengths. + // This reduces regex-based 'unstyled' calls by 50% and avoids redundant O(Cols * Rows) array mapping. + const unstyledLengths = lines.map((line) => { + const lineLengths = new Array(line.length) + for (let i = 0; i < line.length; i++) { + const length = unstyled(line[i]!).length + lineLengths[i] = length + if (i < numColumns && length > widths[i]!) { + widths[i] = length + } + } + return lineLengths + }) + const paddedLines = lines - .map((line) => { + .map((line, rowIndex) => { + const rowLengths = unstyledLengths[rowIndex]! return line - .map((col, index) => { - return `${col}${' '.repeat(widths[index]! - unstyled(col).length)}` + .map((col, colIndex) => { + const cellWidth = rowLengths[colIndex]! + const columnWidth = widths[colIndex] ?? cellWidth + const padding = ' '.repeat(columnWidth - cellWidth) + return `${col}${padding}` }) .join(' ') .trimEnd() diff --git a/packages/cli-kit/src/public/node/hooks/postrun.ts b/packages/cli-kit/src/public/node/hooks/postrun.ts index 09146d0cd8c..52d600cdcbc 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.ts @@ -2,7 +2,6 @@ * Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics) * at module evaluation time. These are only needed after the command has already finished. */ -import {treeKill} from '../tree-kill.js' import {Command, Hook} from '@oclif/core' let postRunHookCompleted = false @@ -16,42 +15,6 @@ export function postRunHookHasCompleted(): boolean { return postRunHookCompleted } -/** - * Wait for the postrun hook to finish (so auto-upgrade has a chance to run) and then - * tree-kill the current process tree before exiting. - * - * Long-running interactive commands like `app dev` need this when the user terminates - * the command via `q` or Ctrl+C. The dev sub-processes such as servers and watchers keep - * the event loop alive, so even after oclif's postrun hook completes the node process - * won't exit on its own and we have to `treeKill` the process tree. We must not do that - * before the postrun hook has actually finished running auto-upgrade, otherwise we would - * kill the upgrade mid-way while `npm install` is still running. - * - * The flag `postRunHookCompleted` is flipped at the very end of the hook after - * `autoUpgradeIfNeeded` resolves, so polling it here is safe. - */ -export function waitForPostRunHookAndExit(): void { - const pollIntervalMs = 100 - // Auto-upgrade can take a while (npm/pnpm/yarn install). Cap the wait generously so - // a stuck upgrade still terminates the process eventually. - const maxWaitMs = 120000 - - let elapsed = 0 - let terminating = false - const handle = setInterval(() => { - if (terminating) return - if (postRunHookHasCompleted() || elapsed >= maxWaitMs) { - terminating = true - clearInterval(handle) - treeKill(process.pid, 'SIGINT', false, () => { - process.exit(0) - }) - return - } - elapsed += pollIntervalMs - }, pollIntervalMs) -} - // This hook is called after each successful command run. More info: https://oclif.io/docs/hooks export const hook: Hook.Postrun = async ({config, Command}) => { await detectStopCommand(Command as unknown as typeof Command) @@ -65,9 +28,9 @@ export const hook: Hook.Postrun = async ({config, Command}) => { const {outputDebug} = await import('../output.js') const command = Command.id.replace(/:/g, ' ') outputDebug(`Completed command ${command}`) + postRunHookCompleted = true if (!command.includes('notifications') && !command.includes('upgrade')) await autoUpgradeIfNeeded() - postRunHookCompleted = true } /** diff --git a/packages/cli-kit/src/public/node/is-global.test.ts b/packages/cli-kit/src/public/node/is-global.test.ts index 258315cdc55..4e2e0b91e9e 100644 --- a/packages/cli-kit/src/public/node/is-global.test.ts +++ b/packages/cli-kit/src/public/node/is-global.test.ts @@ -81,23 +81,6 @@ describe('currentProcessIsGlobal', () => { // Then expect(got).toBeFalsy() }) - - test('returns false on Windows when argv uses backslashes and projectDir uses forward slashes', () => { - // Given - mimic the exact Windows shape: - // projectDir comes from pathe -> normalized forward slashes - // argv[1] is OS-native -> backslash separated - const winProjectDir = 'C:/Users/me/project' - const winArgv1 = 'C:\\Users\\me\\project\\node_modules\\@shopify\\cli\\bin\\run.js' - vi.mocked(findPathUpSync).mockReturnValue(`${winProjectDir}/shopify.app.toml`) - const argv = ['node', winArgv1, 'shopify'] - - // When - const got = currentProcessIsGlobal(argv) - - // Then - regression test for the path-separator bug that misclassified - // Windows local installs as global, triggering an unwanted `npm install -g`. - expect(got).toBeFalsy() - }) }) describe('inferPackageManagerForGlobalCLI', () => { @@ -246,25 +229,6 @@ describe('inferPackageManagerForGlobalCLI', () => { expect(got).toBe('homebrew') }) - test('returns bun when symlink under ~/.bun/bin resolves out of the bun install dir', async () => { - // Given: `bun add -g @shopify/cli` creates ~/.bun/bin/shopify as a symlink to - // ../../node_modules/@shopify/cli/bin/run.js, which on most setups resolves to - // /node_modules/@shopify/cli/bin/run.js — a path that does NOT contain "bun". - // Without inspecting the original symlink path we'd fall through to npm and the - // autoupgrade flow would shell out to `npm install -g` instead of `bun add -g`. - const symlinkPath = '/users/fonso/.bun/bin/shopify' - const realBunPath = '/users/fonso/node_modules/@shopify/cli/bin/run.js' - const argv = ['node', symlinkPath, 'shopify'] - - vi.mocked(realpathSync).mockImplementationOnce(() => realBunPath) - - // When - const got = inferPackageManagerForGlobalCLI(argv) - - // Then - expect(got).toBe('bun') - }) - test('defaults to npm if realpath fails and no other indicator is present', async () => { // Given: A path that realpathSync cannot resolve const nonExistentPath = '/opt/homebrew/bin/shopify' diff --git a/packages/cli-kit/src/public/node/is-global.ts b/packages/cli-kit/src/public/node/is-global.ts index 2a60d6745ba..ac0a1e68958 100644 --- a/packages/cli-kit/src/public/node/is-global.ts +++ b/packages/cli-kit/src/public/node/is-global.ts @@ -1,4 +1,4 @@ -import {cwd, dirname, isSubpath, joinPath, sniffForPath} from './path.js' +import {cwd, dirname, joinPath, sniffForPath} from './path.js' import {isUnitTest} from './context/local.js' import {findPathUpSync, globSync} from './fs.js' import {realpathSync} from 'fs' @@ -27,17 +27,9 @@ export function currentProcessIsGlobal(argv = process.argv): boolean { // From node docs: "The second element [of the array] will be the path to the JavaScript file being executed" const binDir = argv[1] ?? '' - if (!binDir) { - return true - } - // If binDir lives inside projectDir, we are running a local CLI. - // Use isSubpath (pathe.relative under the hood) instead of a raw - // string startsWith: projectDir flows through normalizePath and is - // forward-slash on every platform, while argv[1] is OS-native, so on - // Windows it arrives backslash-separated and a naive startsWith would - // misclassify a local install as global. - const isLocal = isSubpath(projectDir.trim(), binDir) + // If binDir starts with projectDir, then we are running a local CLI + const isLocal = binDir.startsWith(projectDir.trim()) _isGlobal = !isLocal return _isGlobal @@ -105,10 +97,9 @@ export function inferPackageManagerForGlobalCLI(argv = process.argv, env = proce } const processArgv = argv[1] ?? '' - const symlinkPath = processArgv.toLowerCase() // Resolve symlinks to get the real path of the binary. - let realPath = symlinkPath + let realPath = processArgv.toLowerCase() try { realPath = realpathSync(processArgv).toLowerCase() // eslint-disable-next-line no-catch-all/no-catch-all @@ -116,16 +107,9 @@ export function inferPackageManagerForGlobalCLI(argv = process.argv, env = proce // fall back to using the original path for detection } - // Inspect both the (unresolved) symlink path and the resolved real path. Some - // package managers — notably bun (`~/.bun/bin/`) — install global binaries - // as symlinks pointing into a generic `node_modules` directory whose real path - // no longer contains the package manager name. The original symlink under the - // PM's bin dir is the most reliable signal in that case. - const matches = (needle: string) => realPath.includes(needle) || symlinkPath.includes(needle) - - if (matches('yarn')) return 'yarn' - if (matches('pnpm')) return 'pnpm' - if (matches('bun')) return 'bun' + if (realPath.includes('yarn')) return 'yarn' + if (realPath.includes('pnpm')) return 'pnpm' + if (realPath.includes('bun')) return 'bun' // Check for Homebrew via Cellar path (resolved symlink) if (realPath.includes('/cellar/')) return 'homebrew' diff --git a/packages/cli-kit/src/public/node/ui.tsx b/packages/cli-kit/src/public/node/ui.tsx index 4147ee6247d..35c6bab6a7a 100644 --- a/packages/cli-kit/src/public/node/ui.tsx +++ b/packages/cli-kit/src/public/node/ui.tsx @@ -417,11 +417,6 @@ export async function renderAutocompletePrompt( ): Promise { throwInNonTTY({message: props.message, stdin: renderOptions?.stdin}, uiDebugOptions) - // The default search filters in-memory choices synchronously, so it doesn't need - // throttling. Skipping the throttle makes the keystroke-to-result latency feel - // instant. Callers that supply their own (typically remote/paginated) search keep - // the component's default throttle unless they opt out via `searchDebounceMs`. - const usingDefaultSearch = props.search === undefined const newProps = { search(term: string) { const lowerTerm = term.toLowerCase() @@ -431,7 +426,6 @@ export async function renderAutocompletePrompt( }), }) }, - ...(usingDefaultSearch ? {searchDebounceMs: 0} : {}), ...props, } diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index ba9eb541cf5..4db1da0995b 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -12,7 +12,6 @@ import { getPackageManager, } from './node-package-manager.js' import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from './output.js' -import {renderSuccess} from './ui.js' import {cwd, moduleDirectory, sniffForPath} from './path.js' import {exec, isCI} from './system.js' import {isPreReleaseVersion} from './version.js' @@ -93,21 +92,8 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise if (!command) { throw new Error('Could not determine the command to run') } - // Cache lookup — already populated by versionToAutoUpgrade() upstream, so this - // is just a synchronous cache read, no extra network call. - const newerVersion = checkForCachedNewVersion('@shopify/cli', CLI_KIT_VERSION) - const headline = newerVersion - ? `✨ New version of Shopify CLI available! (${CLI_KIT_VERSION} → ${newerVersion})` - : '✨ New version of Shopify CLI available!' - outputInfo( - outputContent`${headline} - Now upgrading by running: ${outputToken.genericShellCommand(installCommand)}...`, - ) + outputInfo(outputContent`Upgrading Shopify CLI by running: ${outputToken.genericShellCommand(installCommand)}...`) await exec(command, args, {stdio: 'inherit'}) - renderSuccess({ - headline: 'Shopify CLI upgraded.', - body: newerVersion ? `You're now on version ${newerVersion}.` : "You're now on the latest version.", - }) } else if (projectDir) { await upgradeLocalShopify(projectDir, CLI_KIT_VERSION) } else {