From 98607ccdb061920253b9aaf8b5d948937dc6e351 Mon Sep 17 00:00:00 2001 From: Mitch Lillie Date: Wed, 15 Apr 2026 13:37:48 -0700 Subject: [PATCH] Fix web/extension build ordering for deploy and dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extensions like the admin module copy output from web builds (e.g. dist/) into the bundle via include_assets. Both deploy and dev ran web builds concurrently with extension builds, causing a race condition where dist/ was empty or missing when the admin extension tried to copy it. Deploy: split the single renderConcurrent call into two sequential phases — web builds first, then extension builds. Dev: wait for the web dev process to populate static_root before building extensions. We cannot run commands.build ourselves because it races with the concurrent web dev process on the same output directory. Fixes: index_missing: index.html must be present in the bundle. --- .../src/cli/services/deploy/bundle.test.ts | 16 ++- .../app/src/cli/services/deploy/bundle.ts | 10 +- .../dev/app-events/app-event-watcher.test.ts | 102 +++++++++++++++++- .../dev/app-events/app-event-watcher.ts | 60 ++++++++++- 4 files changed, 183 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/services/deploy/bundle.test.ts b/packages/app/src/cli/services/deploy/bundle.test.ts index e30d3e091f..567b71d1e6 100644 --- a/packages/app/src/cli/services/deploy/bundle.test.ts +++ b/packages/app/src/cli/services/deploy/bundle.test.ts @@ -256,15 +256,26 @@ describe('bundleAndBuildExtensions', () => { }) }) - test('runs web build command concurrently with extensions when build command is defined', async () => { + test('runs web build before extensions when build command is defined', async () => { await file.inTemporaryDirectory(async (tmpDir: string) => { // Given const bundlePath = joinPath(tmpDir, 'bundle.zip') const mockBuildWeb = vi.mocked(webService.default) + // Track ordering: web build must finish before extension build starts + const callOrder: string[] = [] + mockBuildWeb.mockImplementation(async () => { + callOrder.push('web-build-start') + // Simulate async work + await new Promise((resolve) => setTimeout(resolve, 10)) + callOrder.push('web-build-end') + }) + const functionExtension = await testFunctionExtension() const extensionBuildMock = vi.fn().mockImplementation(async (options, bundleDirectory) => { + callOrder.push('extension-build-start') file.writeFileSync(joinPath(bundleDirectory, 'index.wasm'), '') + callOrder.push('extension-build-end') }) functionExtension.buildForBundle = extensionBuildMock @@ -300,8 +311,9 @@ describe('bundleAndBuildExtensions', () => { isDevDashboardApp: false, }) - // Then + // Then — web build must complete before any extension build starts expect(mockBuildWeb).toHaveBeenCalledWith('build', expect.objectContaining({web: app.webs[0]})) + expect(callOrder).toEqual(['web-build-start', 'web-build-end', 'extension-build-start', 'extension-build-end']) }) }) diff --git a/packages/app/src/cli/services/deploy/bundle.ts b/packages/app/src/cli/services/deploy/bundle.ts index 5811ff368a..c071d12e1b 100644 --- a/packages/app/src/cli/services/deploy/bundle.ts +++ b/packages/app/src/cli/services/deploy/bundle.ts @@ -68,8 +68,16 @@ export async function bundleAndBuildExtensions(options: BundleOptions) { }, })) + // Web builds must complete before extension builds start. Extensions like + // the admin module copy output from web builds (e.g. dist/) into the bundle + // via include_assets. Running them in parallel causes a race condition where + // dist/ is empty or missing when the admin extension tries to copy it. + if (webBuildProcesses.length > 0) { + await renderConcurrent({processes: webBuildProcesses, showTimestamps: false}) + } + await renderConcurrent({ - processes: [webBuildProcesses, extensionBuildProcesses].flat(), + processes: extensionBuildProcesses, showTimestamps: false, }) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts index 93364949b1..f9f3ca7e7b 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts @@ -11,12 +11,13 @@ import { import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {loadApp, reloadApp} from '../../../models/app/loader.js' import {AppLinkedInterface, CurrentAppConfiguration} from '../../../models/app/app.js' +import {loadLocalExtensionsSpecifications} from '../../../models/extensions/load-specifications.js' import {AppAccessSpecIdentifier} from '../../../models/extensions/specifications/app_config_app_access.js' import {PosSpecIdentifier} from '../../../models/extensions/specifications/app_config_point_of_sale.js' import {afterEach, beforeEach, describe, expect, test, vi, type MockInstance} from 'vitest' import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort' import {flushPromises} from '@shopify/cli-kit/node/promises' -import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, mkdir, touchFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {Writable} from 'stream' @@ -429,6 +430,105 @@ describe('app-event-watcher', () => { ) }) + describe('waitForStaticRoots', () => { + test('waits for static_root directory to be populated before building extensions', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') + + // Create an admin extension with static_root pointing to ./dist + const allSpecs = await loadLocalExtensionsSpecifications() + const adminSpec = allSpecs.find((spec) => spec.identifier === 'admin')! + const adminExtension = new ExtensionInstance({ + configuration: {admin: {static_root: './dist'}} as any, + configurationPath: joinPath(tmpDir, 'shopify.app.toml'), + directory: tmpDir, + specification: adminSpec, + }) + vi.spyOn(adminExtension, 'buildForBundle').mockResolvedValue() + + const app = testAppLinked({ + allExtensions: [adminExtension], + configuration: testAppConfiguration, + }) + + // Simulate the web dev process creating dist/ after a delay + setTimeout(() => { + const distDir = joinPath(tmpDir, 'dist') + mkdir(distDir) + .then(() => touchFile(joinPath(distDir, 'index.html'))) + .catch(() => {}) + }, 300) + + const mockFileWatcher = new MockFileWatcher(app, outputOptions, []) + const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher) + await watcher.start({stdout, stderr, signal: abortController.signal}) + + // Extension should have been built (after waiting for dist/) + expect(adminExtension.buildForBundle).toHaveBeenCalled() + }) + }) + + test('proceeds immediately when static_root already has files', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') + + // Pre-create dist/ with files + const distDir = joinPath(tmpDir, 'dist') + await mkdir(distDir) + await touchFile(joinPath(distDir, 'index.html')) + + const allSpecs = await loadLocalExtensionsSpecifications() + const adminSpec = allSpecs.find((spec) => spec.identifier === 'admin')! + const adminExtension = new ExtensionInstance({ + configuration: {admin: {static_root: './dist'}} as any, + configurationPath: joinPath(tmpDir, 'shopify.app.toml'), + directory: tmpDir, + specification: adminSpec, + }) + vi.spyOn(adminExtension, 'buildForBundle').mockResolvedValue() + + const app = testAppLinked({ + allExtensions: [adminExtension], + configuration: testAppConfiguration, + }) + + const mockFileWatcher = new MockFileWatcher(app, outputOptions, []) + const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher) + + const startTime = Date.now() + await watcher.start({stdout, stderr, signal: abortController.signal}) + const elapsed = Date.now() - startTime + + // Should not have waited — proceeds immediately + expect(elapsed).toBeLessThan(1000) + expect(adminExtension.buildForBundle).toHaveBeenCalled() + }) + }) + + test('skips waiting when no admin extension has static_root', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') + + // extension1 is a UI extension, no static_root + const app = testAppLinked({ + allExtensions: [extension1], + configuration: testAppConfiguration, + }) + + const mockFileWatcher = new MockFileWatcher(app, outputOptions, []) + const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockFileWatcher) + + const startTime = Date.now() + await watcher.start({stdout, stderr, signal: abortController.signal}) + const elapsed = Date.now() - startTime + + // Should not have waited at all + expect(elapsed).toBeLessThan(1000) + expect(extension1.buildForBundle).toHaveBeenCalled() + }) + }) + }) + describe('generateExtensionTypes', () => { test('is called after extensions are rebuilt on file changes', async () => { await inTemporaryDirectory(async (tmpDir) => { diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index 5bda637f46..72b84071c1 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -4,15 +4,44 @@ import {handleWatcherEvents} from './app-event-watcher-handler.js' import {AppLinkedInterface} from '../../../models/app/app.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {ExtensionBuildOptions} from '../../build/extension.js' +import {AdminSpecIdentifier, type AdminConfigType} from '../../../models/extensions/specifications/admin.js' import {outputDebug} from '@shopify/cli-kit/node/output' import {AbortSignal} from '@shopify/cli-kit/node/abort' +import {sleep} from '@shopify/cli-kit/node/system' import {joinPath} from '@shopify/cli-kit/node/path' -import {fileExists, mkdir, rmdir} from '@shopify/cli-kit/node/fs' +import {fileExists, mkdir, rmdir, readdir} from '@shopify/cli-kit/node/fs' import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' import {groupBy} from '@shopify/cli-kit/common/collection' import {formatMessagesSync, Message} from 'esbuild' import {isUnitTest} from '@shopify/cli-kit/node/context/local' import EventEmitter from 'events' +import {Writable} from 'stream' + +const POLL_TIMEOUT_MS = 30_000 +const POLL_INTERVAL_S = 0.2 + +/** + * Polls until a directory exists and contains at least one file. + * Resolves once files are found or the timeout expires (no error on timeout — + * the subsequent include_assets step will log a warning for the missing path). + */ +/* eslint-disable no-await-in-loop */ +async function pollForDirectory(dirPath: string, label: string, stdout: Writable): Promise { + outputDebug(`Waiting for '${label}' to be populated by web dev process...\n`, stdout) + + const deadline = Date.now() + POLL_TIMEOUT_MS + while (Date.now() < deadline) { + if (await fileExists(dirPath)) { + const files = await readdir(dirPath) + if (files.length > 0) { + outputDebug(`Found files in '${label}'\n`, stdout) + return + } + } + await sleep(POLL_INTERVAL_S) + } +} +/* eslint-enable no-await-in-loop */ /** This is the entry point to start watching events in an app. This process has 3 steps: @@ -120,6 +149,12 @@ export class AppEventWatcher extends EventEmitter { // Initial build of all extensions if (buildExtensionsFirst) { + // Wait for web dev processes to produce output that extensions need. + // Extensions like admin copy from web output (e.g. dist/) via include_assets. + // The web dev process runs concurrently and creates these files, so we + // must wait for them before building extensions that depend on them. + await this.waitForStaticRoots() + this.initialEvents = this.app.realExtensions.map((ext) => ({type: EventType.Updated, extension: ext})) await this.buildExtensions(this.initialEvents) } @@ -277,6 +312,29 @@ export class AppEventWatcher extends EventEmitter { return Promise.all(promises) } + /** + * Waits for static_root directories referenced by admin extensions to be + * populated. The web dev process (commands.dev) runs as a sibling concurrent + * process and produces these files (e.g. dist/index.html). We cannot run + * commands.build ourselves because it races with the concurrent dev process + * on the same output directory. + * + * Polls until the directory exists and contains at least one file, or until + * the timeout expires. No-op when no admin extension references a static_root. + */ + private async waitForStaticRoots(): Promise { + const waits = this.app.realExtensions + .filter((ext): ext is ExtensionInstance => ext.specification.identifier === AdminSpecIdentifier) + .filter((ext) => ext.configuration.admin?.static_root !== undefined) + .map((ext) => { + const staticRoot = ext.configuration.admin!.static_root! + const fullPath = joinPath(ext.directory, staticRoot) + return pollForDirectory(fullPath, staticRoot, this.options.stdout) + }) + + await Promise.all(waits) + } + /** * Build a single extension using the default buildForBundle method. * @param extension - The extension to build