diff --git a/packages/theme/src/cli/polyfills/promiseWithResolvers.ts b/packages/theme/src/cli/polyfills/promiseWithResolvers.ts new file mode 100644 index 0000000000..35f491107b --- /dev/null +++ b/packages/theme/src/cli/polyfills/promiseWithResolvers.ts @@ -0,0 +1,22 @@ +interface PromiseWithResolvers { + promise: Promise + resolve: (value: T | PromiseLike) => void + reject: (reason?: unknown) => void +} + +// Polyfill for Promise.withResolvers +// Can remove once our minimum supported Node version is 22 +export function promiseWithResolvers(): PromiseWithResolvers { + if (typeof Promise.withResolvers === 'function') { + return Promise.withResolvers() + } + + let resolve!: (value: T | PromiseLike) => void + let reject!: (reason?: unknown) => void + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve + reject = _reject + }) + + return {promise, resolve, reject} +} diff --git a/packages/theme/src/cli/services/dev.ts b/packages/theme/src/cli/services/dev.ts index 75d77e7c89..e223e8159e 100644 --- a/packages/theme/src/cli/services/dev.ts +++ b/packages/theme/src/cli/services/dev.ts @@ -119,23 +119,7 @@ export async function dev(options: DevOptions) { }, } - if (options['theme-editor-sync']) { - session.storefrontPassword = await storefrontPasswordPromise - } - - const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx) - - if (!options['theme-editor-sync']) { - session.storefrontPassword = await storefrontPasswordPromise - } - - await renderDevSetupProgress() - await serverStart() - - renderLinks(urls) - if (options.open) { - openURLSafely(urls.local, 'development server') - } + const {serverStart, renderDevSetupProgress, backgroundJobPromise} = setupDevServer(options.theme, ctx) readline.emitKeypressEvents(process.stdin) if (process.stdin.isTTY) { @@ -167,6 +151,18 @@ export async function dev(options: DevOptions) { break } }) + + await Promise.all([ + backgroundJobPromise, + renderDevSetupProgress() + .then(serverStart) + .then(() => { + renderLinks(urls) + if (options.open) { + openURLSafely(urls.local, 'development server') + } + }), + ]) } export function openURLSafely(url: string, label: string) { diff --git a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts index f34edcfea6..a37cbeb83c 100644 --- a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts @@ -28,6 +28,7 @@ describe('reconcileAndPollThemeEditorChanges', async () => { const files = new Map([]) const defaultThemeFileSystem = fakeThemeFileSystem('tmp', files) const initialRemoteChecksums = [{checksum: '1', key: 'templates/asset.json'}] + const mockRejectBackgroundJob = vi.fn() vi.mocked(fetchChecksums).mockResolvedValue([{checksum: '2', key: 'templates/asset.json'}]) @@ -42,6 +43,7 @@ describe('reconcileAndPollThemeEditorChanges', async () => { ignore: [], only: [], }, + mockRejectBackgroundJob, ) await workPromise await updatedRemoteChecksumsPromise @@ -57,6 +59,7 @@ describe('reconcileAndPollThemeEditorChanges', async () => { ignore: [], only: [], }, + mockRejectBackgroundJob, ) }) @@ -65,6 +68,7 @@ describe('reconcileAndPollThemeEditorChanges', async () => { const files = new Map([]) const defaultThemeFileSystem = fakeThemeFileSystem('tmp', files) const initialRemoteChecksums = [{checksum: '1', key: 'templates/asset.json'}] + const mockRejectBackgroundJob = vi.fn() const options = { noDelete: false, ignore: [], @@ -83,6 +87,7 @@ describe('reconcileAndPollThemeEditorChanges', async () => { initialRemoteChecksums, defaultThemeFileSystem, options, + mockRejectBackgroundJob, ) // Then diff --git a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts index a8d7470b73..7e129d2034 100644 --- a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts +++ b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts @@ -22,6 +22,7 @@ export async function reconcileAndPollThemeEditorChanges( ignore: string[] only: string[] }, + rejectBackgroundJob: (reason?: unknown) => void, ): Promise<{ updatedRemoteChecksumsPromise: Promise workPromise: Promise @@ -33,7 +34,14 @@ export async function reconcileAndPollThemeEditorChanges( const updatedRemoteChecksumsPromise = workPromise.then(async () => { const updatedRemoteChecksums = await fetchChecksums(targetTheme.id, session) - pollThemeEditorChanges(targetTheme, session, updatedRemoteChecksums, localThemeFileSystem, options) + pollThemeEditorChanges( + targetTheme, + session, + updatedRemoteChecksums, + localThemeFileSystem, + options, + rejectBackgroundJob, + ) return updatedRemoteChecksums }) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index 9cc63555ba..195e7316c9 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -11,10 +11,11 @@ import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest' import {buildTheme} from '@shopify/cli-kit/node/themes/factories' import {createEvent} from 'h3' import * as output from '@shopify/cli-kit/node/output' +import {fetchChecksums} from '@shopify/cli-kit/node/themes/api' import {IncomingMessage, ServerResponse} from 'node:http' import {Socket} from 'node:net' -vi.mock('@shopify/cli-kit/node/themes/api', () => ({fetchChecksums: () => Promise.resolve([])})) +vi.mock('@shopify/cli-kit/node/themes/api', () => ({fetchChecksums: vi.fn(() => Promise.resolve([]))})) vi.mock('./remote-theme-watcher.js') vi.mock('./storefront-renderer.js') vi.spyOn(output, 'outputDebug') @@ -158,7 +159,15 @@ describe('setupDevServer', () => { [], context.localThemeFileSystem, {noDelete: true, ...filters}, + expect.anything(), ) + // This is the best way I could think of verifying the rejectBackgroundJob + // Verify the rejectBackgroundJob callback is a function accepting one argument + const callArgs = vi.mocked(reconcileAndPollThemeEditorChanges).mock.calls[0] + const rejectCallback = callArgs?.[5] + expect(rejectCallback).toBeTypeOf('function') + // Reject callbacks take 1 argument: the rejection reason + expect(rejectCallback).toHaveLength(1) }) test('should skip deletion of remote files if noDelete flag is passed', async () => { @@ -179,6 +188,22 @@ describe('setupDevServer', () => { }) }) + test('should catch errors from fetchChecksums and reject backgroundJobPromise', async () => { + // Given + const context: DevServerContext = { + ...defaultServerContext, + } + const expectedError = new Error('Failed to fetch checksums from API') + + vi.mocked(fetchChecksums).mockRejectedValueOnce(expectedError) + + // When + const {backgroundJobPromise} = setupDevServer(developmentTheme, context) + + // Then + await expect(backgroundJobPromise).rejects.toThrow('Failed to fetch checksums from API') + }) + describe('request handling', async () => { const context = {...defaultServerContext} const server = setupDevServer(developmentTheme, context) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts index 0504aa81d7..34bbd14d71 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts @@ -5,7 +5,8 @@ import {getProxyHandler} from './proxy.js' import {reconcileAndPollThemeEditorChanges} from './remote-theme-watcher.js' import {uploadTheme} from '../theme-uploader.js' import {renderTasksToStdErr} from '../theme-ui.js' -import {createAbortCatchError} from '../errors.js' +import {renderThrownError} from '../errors.js' +import {promiseWithResolvers} from '../../polyfills/promiseWithResolvers.js' import {createApp, defineEventHandler, defineLazyEventHandler, toNodeListener, handleCors} from 'h3' import {fetchChecksums} from '@shopify/cli-kit/node/themes/api' import {createServer} from 'node:http' @@ -13,8 +14,10 @@ import type {Checksum, Theme} from '@shopify/cli-kit/node/themes/types' import type {DevServerContext} from './types.js' export function setupDevServer(theme: Theme, ctx: DevServerContext) { + const {promise: backgroundJobPromise, reject: rejectBackgroundJob} = promiseWithResolvers() + const watcherPromise = setupInMemoryTemplateWatcher(theme, ctx) - const envSetup = ensureThemeEnvironmentSetup(theme, ctx) + const envSetup = ensureThemeEnvironmentSetup(theme, ctx, rejectBackgroundJob) const workPromise = Promise.all([watcherPromise, envSetup.workPromise]).then(() => ctx.localThemeFileSystem.startWatcher(theme.id.toString(), ctx.session), ) @@ -25,31 +28,43 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) { serverStart: server.start, dispatchEvent: server.dispatch, renderDevSetupProgress: envSetup.renderProgress, + backgroundJobPromise, } } -function ensureThemeEnvironmentSetup(theme: Theme, ctx: DevServerContext) { - const abort = createAbortCatchError('Failed to perform the initial theme synchronization.') +function ensureThemeEnvironmentSetup( + theme: Theme, + ctx: DevServerContext, + rejectBackgroundJob: (reason?: unknown) => void, +) { + const abort = (error: Error): never => { + renderThrownError('Failed to perform the initial theme synchronization.', error) + rejectBackgroundJob(error) + // Return a never-resolving promise to stop this promise chain without throwing. + // Throwing would trigger catch handlers and continue execution. This stops the + // chain while the error is handled through the separate backgroundJobPromise channel. + return new Promise(() => {}) as never + } - const remoteChecksumsPromise = fetchChecksums(theme.id, ctx.session).catch(abort) + const remoteChecksumsPromise = fetchChecksums(theme.id, ctx.session) - const reconcilePromise = remoteChecksumsPromise - .then((remoteChecksums) => handleThemeEditorSync(theme, ctx, remoteChecksums)) - .catch(abort) + const reconcilePromise = remoteChecksumsPromise.then((remoteChecksums) => + handleThemeEditorSync(theme, ctx, remoteChecksums, rejectBackgroundJob), + ) - const uploadPromise = reconcilePromise - .then(async ({updatedRemoteChecksumsPromise}) => { - const updatedRemoteChecksums = await updatedRemoteChecksumsPromise - return uploadTheme(theme, ctx.session, updatedRemoteChecksums, ctx.localThemeFileSystem, { - nodelete: ctx.options.noDelete, - deferPartialWork: true, - backgroundWorkCatch: abort, - }) + const uploadPromise = reconcilePromise.then(async ({updatedRemoteChecksumsPromise}) => { + const updatedRemoteChecksums = await updatedRemoteChecksumsPromise + return uploadTheme(theme, ctx.session, updatedRemoteChecksums, ctx.localThemeFileSystem, { + nodelete: ctx.options.noDelete, + deferPartialWork: true, + backgroundWorkCatch: abort, }) - .catch(abort) + }) + + const workPromise = uploadPromise.then((result) => result.workPromise).catch(abort) return { - workPromise: uploadPromise.then((result) => result.workPromise).catch(abort), + workPromise, renderProgress: async () => { if (ctx.options.themeEditorSync) { const {workPromise} = await reconcilePromise @@ -74,16 +89,24 @@ function handleThemeEditorSync( theme: Theme, ctx: DevServerContext, remoteChecksums: Checksum[], + rejectBackgroundJob: (reason?: unknown) => void, ): Promise<{ updatedRemoteChecksumsPromise: Promise workPromise: Promise }> { if (ctx.options.themeEditorSync) { - return reconcileAndPollThemeEditorChanges(theme, ctx.session, remoteChecksums, ctx.localThemeFileSystem, { - noDelete: ctx.options.noDelete, - ignore: ctx.options.ignore, - only: ctx.options.only, - }) + return reconcileAndPollThemeEditorChanges( + theme, + ctx.session, + remoteChecksums, + ctx.localThemeFileSystem, + { + noDelete: ctx.options.noDelete, + ignore: ctx.options.ignore, + only: ctx.options.only, + }, + rejectBackgroundJob, + ) } else { return Promise.resolve({ updatedRemoteChecksumsPromise: Promise.resolve(remoteChecksums), diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts b/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts index 4ecf2f79f4..342e0f2a9b 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-polling.ts @@ -21,6 +21,7 @@ export function pollThemeEditorChanges( remoteChecksum: Checksum[], localFileSystem: ThemeFileSystem, options: PollingOptions, + rejectBackgroundJob: (reason?: unknown) => void, ) { outputDebug('Listening for changes in the theme editor') @@ -51,14 +52,12 @@ export function pollThemeEditorChanges( } if (failedPollingAttempts >= maxPollingAttempts) { - renderFatalError( - new AbortError( - 'Too many polling errors...', - 'Please check the errors above and ensure you have a stable internet connection.', - ), + const fatalError = new AbortError( + 'Too many polling errors...', + 'Please check the errors above and ensure you have a stable internet connection.', ) - - process.exit(1) + renderFatalError(fatalError) + rejectBackgroundJob(fatalError) } return latestChecksums