From 053c3317997c37a311448e6e750992f1f20edc72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 20 Nov 2024 13:27:22 +0100 Subject: [PATCH 1/2] Keep a copy of built artifacts in each extension folder --- .../dev/app-events/app-event-watcher.ts | 2 +- .../dev/app-events/app-watcher-esbuild.ts | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) 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 d2f4c16ac5..a1c4b670b3 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 @@ -223,7 +223,7 @@ export class AppEventWatcher extends EventEmitter { return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { try { if (this.esbuildManager.contexts[ext.handle]) { - await this.esbuildManager.contexts[ext.handle]?.rebuild() + await this.esbuildManager.rebuildContext(ext) } else { await this.buildExtension(ext) } diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts index fec1fab652..2dc89991fb 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts @@ -3,6 +3,8 @@ import {ExtensionInstance} from '../../../models/extensions/extension-instance.j import {getESBuildOptions} from '../../extensions/bundle.js' import {BuildContext, context as esContext} from 'esbuild' import {AbortSignal} from '@shopify/cli-kit/node/abort' +import {copyFile} from '@shopify/cli-kit/node/fs' +import {dirname} from '@shopify/cli-kit/node/path' export interface DevAppWatcherOptions { dotEnvVariables: {[key: string]: string} @@ -65,6 +67,22 @@ export class ESBuildContextManager { await Promise.all(promises) } + async rebuildContext(extension: ExtensionInstance) { + const context = this.contexts[extension.handle] + if (!context) return + await context.rebuild() + + // The default output path for a extension is now inside `.shopify/bundle//dist`, + // all extensions output need to be under the same directory so that it can all be zipped together. + + // But historically the output was inside each extension's directory. + // To avoid breaking flows that depend on this, we copy the output to the old location. + // This also makes it easier to access sourcemaps or other built artifacts. + const outputPath = dirname(extension.getOutputPathForDirectory(this.outputPath)) + const copyPath = dirname(extension.outputPath) + await copyFile(outputPath, copyPath) + } + async updateContexts(appEvent: AppEvent) { this.dotEnvVariables = appEvent.app.dotenv?.variables ?? {} const createdEsBuild = appEvent.extensionEvents From 838279a61349be6b7ee09ac1b1775b0d6da12028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 20 Nov 2024 15:31:32 +0100 Subject: [PATCH 2/2] add tests --- .../dev/app-events/app-event-watcher.test.ts | 2 +- .../app-events/app-watcher-esbuild.test.ts | 37 +++++++++++-------- 2 files changed, 23 insertions(+), 16 deletions(-) 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 64cf293218..eb1fbeb486 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 @@ -323,7 +323,7 @@ describe('app-event-watcher build extension errors', () => { } const mockManager = new MockESBuildContextManager() - mockManager.contexts.h1.rebuild.mockRejectedValueOnce(esbuildError) + mockManager.rebuildContext = vi.fn().mockRejectedValueOnce(esbuildError) const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts index e2ab43b853..800f6d78c5 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts @@ -2,6 +2,7 @@ import {DevAppWatcherOptions, ESBuildContextManager} from './app-watcher-esbuild import {AppEvent, EventType} from './app-event-watcher.js' import {testAppLinked, testUIExtension} from '../../../models/app/app.test-data.js' import {describe, expect, test, vi} from 'vitest' +import * as fs from '@shopify/cli-kit/node/fs' vi.mock('@luckycatfactory/esbuild-graphql-loader', () => ({ default: { @@ -15,13 +16,14 @@ const extension1 = await testUIExtension({type: 'ui_extension', handle: 'h1', di const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2'}) describe('app-watcher-esbuild', () => { + const options: DevAppWatcherOptions = { + dotEnvVariables: {key: 'value'}, + url: 'http://localhost:3000', + outputPath: '/path/to/output', + } + test('creating contexts', async () => { // Given - const options: DevAppWatcherOptions = { - dotEnvVariables: {key: 'value'}, - url: 'http://localhost:3000', - outputPath: '/path/to/output', - } const manager = new ESBuildContextManager(options) const extensions = [extension1, extension2] @@ -35,11 +37,6 @@ describe('app-watcher-esbuild', () => { test('deleting contexts', async () => { // Given - const options: DevAppWatcherOptions = { - dotEnvVariables: {key: 'value'}, - url: 'http://localhost:3000', - outputPath: '/path/to/output', - } const manager = new ESBuildContextManager(options) const extensions = [extension1, extension2] await manager.createContexts(extensions) @@ -54,11 +51,6 @@ describe('app-watcher-esbuild', () => { test('updating contexts with an app event', async () => { // Given - const options: DevAppWatcherOptions = { - dotEnvVariables: {key: 'value'}, - url: 'http://localhost:3000', - outputPath: '/path/to/output', - } const manager = new ESBuildContextManager(options) await manager.createContexts([extension2]) @@ -79,4 +71,19 @@ describe('app-watcher-esbuild', () => { expect(manager.contexts).toHaveProperty('h1') expect(manager.contexts).not.toHaveProperty('test-ui-extension') }) + + test('rebuilding contexts', async () => { + // Given + const manager = new ESBuildContextManager(options) + await manager.createContexts([extension1]) + const spyContext = vi.spyOn(manager.contexts.h1!, 'rebuild').mockResolvedValue({} as any) + const spyCopy = vi.spyOn(fs, 'copyFile').mockResolvedValue() + + // When + await manager.rebuildContext(extension1) + + // Then + expect(spyContext).toHaveBeenCalled() + expect(spyCopy).toHaveBeenCalledWith('/path/to/output/h1/dist', '/extensions/ui_extension_1/dist') + }) })