From b1ec98ba7d51ef8cdfab63f5514b1c1b7ead1197 Mon Sep 17 00:00:00 2001 From: Mitch Lillie Date: Tue, 14 Apr 2026 15:04:32 -0700 Subject: [PATCH] Wait for index.html before copying admin static_root during dev When running 'shopify app dev' immediately after 'shopify app init', there's a race condition where the web build process hasn't completed before the admin extension tries to copy files from static_root. This causes the dev session to fail with 'index.html must be present in the bundle' error. This fix adds a new 'wait_for_file' build step that: - Waits up to 60 seconds for index.html to appear in static_root - Polls every 500ms until the file exists - Only waits if static_root is configured - Succeeds immediately if the file already exists The wait step runs before the include_assets step in the admin extension's build pipeline, ensuring the files are ready before copying. Fixes: https://github.com/shop/issues-admin-extensibility/issues/2411 --- .../models/extensions/specifications/admin.ts | 11 ++ .../src/cli/services/build/client-steps.ts | 9 +- .../app/src/cli/services/build/steps/index.ts | 4 + .../build/steps/wait-for-file-step.test.ts | 126 ++++++++++++++++++ .../build/steps/wait-for-file-step.ts | 100 ++++++++++++++ 5 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 packages/app/src/cli/services/build/steps/wait-for-file-step.test.ts create mode 100644 packages/app/src/cli/services/build/steps/wait-for-file-step.ts diff --git a/packages/app/src/cli/models/extensions/specifications/admin.ts b/packages/app/src/cli/models/extensions/specifications/admin.ts index bdd20cb2131..8b4897b6713 100644 --- a/packages/app/src/cli/models/extensions/specifications/admin.ts +++ b/packages/app/src/cli/models/extensions/specifications/admin.ts @@ -49,6 +49,17 @@ const adminSpecificationSpec = createExtensionSpecification({ { lifecycle: 'deploy', steps: [ + { + id: 'wait_for_index_html', + name: 'Wait for index.html', + type: 'wait_for_file', + config: { + configKey: 'admin.static_root', + filename: 'index.html', + timeoutMs: 60000, + intervalMs: 500, + }, + }, { id: 'hosted_app_copy_files', name: 'Hosted App Copy Files', diff --git a/packages/app/src/cli/services/build/client-steps.ts b/packages/app/src/cli/services/build/client-steps.ts index 27bb601bd46..d976992fb79 100644 --- a/packages/app/src/cli/services/build/client-steps.ts +++ b/packages/app/src/cli/services/build/client-steps.ts @@ -1,5 +1,6 @@ import {executeStepByType} from './steps/index.js' import type {IncludeAssetsConfig} from './steps/include-assets-step.js' +import type {WaitForFileConfig} from './steps/wait-for-file-step.js' import type {ExtensionInstance} from '../../models/extensions/extension-instance.js' import type {ExtensionBuildOptions} from './extension.js' @@ -21,6 +22,12 @@ interface IncludeAssetsStep extends BaseStep { readonly config: IncludeAssetsConfig } +/** Step with typed config specific to wait_for_file. */ +interface WaitForFileStep extends BaseStep { + readonly type: 'wait_for_file' + readonly config: WaitForFileConfig +} + /** Steps that don't require any config yet. */ interface NoConfigStep extends BaseStep { readonly type: @@ -40,7 +47,7 @@ interface NoConfigStep extends BaseStep { * This is a discriminated union on `type`: each step type carries its own * typed `config`, so TypeScript catches config typos at compile time. */ -export type LifecycleStep = IncludeAssetsStep | NoConfigStep +export type LifecycleStep = IncludeAssetsStep | WaitForFileStep | NoConfigStep /** * A group of steps scoped to a specific lifecycle phase. diff --git a/packages/app/src/cli/services/build/steps/index.ts b/packages/app/src/cli/services/build/steps/index.ts index 6ca10e6e041..3a1c6eebb12 100644 --- a/packages/app/src/cli/services/build/steps/index.ts +++ b/packages/app/src/cli/services/build/steps/index.ts @@ -5,6 +5,7 @@ import {executeBundleUIStep} from './bundle-ui-step.js' import {executeCopyStaticAssetsStep} from './copy-static-assets-step.js' import {executeBuildFunctionStep} from './build-function-step.js' import {executeCreateTaxStubStep} from './create-tax-stub-step.js' +import {executeWaitForFileStep, WaitForFileStep} from './wait-for-file-step.js' import type {LifecycleStep, BuildContext} from '../client-steps.js' /** @@ -21,6 +22,9 @@ export async function executeStepByType(step: LifecycleStep, context: BuildConte case 'include_assets': return executeIncludeAssetsStep(step, context) + case 'wait_for_file': + return executeWaitForFileStep(step as WaitForFileStep, context) + case 'build_theme': return executeBuildThemeStep(step, context) diff --git a/packages/app/src/cli/services/build/steps/wait-for-file-step.test.ts b/packages/app/src/cli/services/build/steps/wait-for-file-step.test.ts new file mode 100644 index 00000000000..5555e58abb2 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/wait-for-file-step.test.ts @@ -0,0 +1,126 @@ +import {executeWaitForFileStep, WaitForFileStep} from './wait-for-file-step.js' +import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' +import {ExtensionBuildOptions} from '../extension.js' +import {inTemporaryDirectory, mkdir, touchFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {describe, expect, test} from 'vitest' +import {Writable} from 'stream' + +function createMockContext(directory: string, staticRoot?: string) { + const stdout = new Writable({ + write(_chunk, _encoding, callback) { + callback() + }, + }) + + return { + extension: { + directory, + configuration: { + name: 'test-admin', + type: 'admin', + admin: staticRoot ? {static_root: staticRoot} : undefined, + }, + } as unknown as ExtensionInstance, + options: { + stdout, + stderr: stdout, + } as unknown as ExtensionBuildOptions, + stepResults: new Map(), + } +} + +function createWaitStep(overrides: Partial = {}): WaitForFileStep { + return { + id: 'test-wait', + name: 'Test Wait Step', + type: 'wait_for_file', + config: { + configKey: 'admin.static_root', + filename: 'index.html', + timeoutMs: 1000, + intervalMs: 100, + ...overrides, + }, + } +} + +describe('wait-for-file-step', () => { + test('succeeds immediately when config key is not set', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const context = createMockContext(tmpDir, undefined) + const step = createWaitStep() + + const result = await executeWaitForFileStep(step, context) + + expect(result.waited).toBe(false) + expect(result.filePath).toBeUndefined() + }) + }) + + test('succeeds immediately when file already exists', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const distDir = joinPath(tmpDir, 'dist') + await mkdir(distDir) + await touchFile(joinPath(distDir, 'index.html')) + + const context = createMockContext(tmpDir, './dist') + const step = createWaitStep() + + const result = await executeWaitForFileStep(step, context) + + expect(result.waited).toBe(false) + expect(result.filePath).toContain('index.html') + }) + }) + + test('waits for file to appear and succeeds', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const distDir = joinPath(tmpDir, 'dist') + await mkdir(distDir) + + const context = createMockContext(tmpDir, './dist') + const step = createWaitStep({timeoutMs: 2000, intervalMs: 50}) + + // Create the file after a short delay + setTimeout(() => { + touchFile(joinPath(distDir, 'index.html')).catch(() => {}) + }, 200) + + const result = await executeWaitForFileStep(step, context) + + expect(result.waited).toBe(true) + expect(result.filePath).toContain('index.html') + }) + }) + + test('times out when file does not appear', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const distDir = joinPath(tmpDir, 'dist') + await mkdir(distDir) + + const context = createMockContext(tmpDir, './dist') + const step = createWaitStep({timeoutMs: 500, intervalMs: 100}) + + await expect(executeWaitForFileStep(step, context)).rejects.toThrow( + "Timed out waiting for 'index.html' in './dist'", + ) + }) + }) + + test('uses custom filename from config', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const distDir = joinPath(tmpDir, 'dist') + await mkdir(distDir) + await touchFile(joinPath(distDir, 'custom.html')) + + const context = createMockContext(tmpDir, './dist') + const step = createWaitStep({filename: 'custom.html'}) + + const result = await executeWaitForFileStep(step, context) + + expect(result.waited).toBe(false) + expect(result.filePath).toContain('custom.html') + }) + }) +}) diff --git a/packages/app/src/cli/services/build/steps/wait-for-file-step.ts b/packages/app/src/cli/services/build/steps/wait-for-file-step.ts new file mode 100644 index 00000000000..271cd4be4e9 --- /dev/null +++ b/packages/app/src/cli/services/build/steps/wait-for-file-step.ts @@ -0,0 +1,100 @@ +import {getNestedValue} from './include-assets/copy-config-key-entry.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileExists} from '@shopify/cli-kit/node/fs' +import {outputDebug} from '@shopify/cli-kit/node/output' +import type {BuildContext} from '../client-steps.js' + +export interface WaitForFileConfig { + /** + * Config key path to resolve the directory (e.g., 'admin.static_root'). + * The directory path is resolved relative to the extension directory. + */ + configKey: string + + /** + * The filename to wait for within the resolved directory. + */ + filename: string + + /** + * Maximum time to wait in milliseconds. + * Default: 60000 (60 seconds) + */ + timeoutMs?: number + + /** + * Interval between checks in milliseconds. + * Default: 500 (0.5 seconds) + */ + intervalMs?: number +} + +export interface WaitForFileStep { + readonly id: string + readonly name: string + readonly type: 'wait_for_file' + readonly config: WaitForFileConfig + readonly continueOnError?: boolean +} + +/** + * Waits for a specific file to exist before proceeding. + * + * This step is useful when the extension depends on files that are built + * asynchronously by another process (e.g., a web process running a build). + * + * If the config key doesn't resolve to a value, the step succeeds immediately + * (the file is not required). + * + * @throws Error if the file doesn't exist within the timeout period + */ +export async function executeWaitForFileStep( + step: WaitForFileStep, + context: BuildContext, +): Promise<{waited: boolean; filePath?: string}> { + const {configKey, filename, timeoutMs = 60000, intervalMs = 500} = step.config + const {stdout} = context.options + + const configValue = getNestedValue(context.extension.configuration, configKey) + + if (typeof configValue !== 'string') { + outputDebug(`No value for configKey '${configKey}', skipping wait\n`, stdout) + return {waited: false} + } + + const filePath = joinPath(context.extension.directory, configValue, filename) + + // Check if file already exists + if (await fileExists(filePath)) { + outputDebug(`File '${filename}' already exists in '${configValue}'\n`, stdout) + return {waited: false, filePath} + } + + stdout.write(`Waiting for '${filename}' in '${configValue}'...\n`) + + const startTime = Date.now() + let elapsed = 0 + + while (elapsed < timeoutMs) { + // eslint-disable-next-line no-await-in-loop + await sleep(intervalMs) + elapsed = Date.now() - startTime + + // eslint-disable-next-line no-await-in-loop + if (await fileExists(filePath)) { + const waitedSeconds = (elapsed / 1000).toFixed(1) + stdout.write(`Found '${filename}' in '${configValue}' (waited ${waitedSeconds}s)\n`) + return {waited: true, filePath} + } + } + + const timeoutSeconds = (timeoutMs / 1000).toFixed(0) + throw new Error( + `Timed out waiting for '${filename}' in '${configValue}' after ${timeoutSeconds}s. ` + + `Make sure your build process creates this file (e.g., via a predev hook).`, + ) +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)) +}