Skip to content

Commit

Permalink
Copy extension build output to legacy location (#4890)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

To maintain backward compatibility with existing extension build processes while transitioning to a new output directory structure under `.shopify/bundle/<ext_id>/dist`.

### WHAT is this pull request doing?

Introduces a file copying mechanism that preserves extension build outputs in their original locations while also storing them in the new centralized directory. This ensures that:
- All extension outputs are available in a single directory for bundling
- Legacy flows depending on the original output locations continue to work
- Source maps and build artifacts remain easily accessible

### How to test your changes?

1. Run `dev` (having ui extensions)
2. Verify that build outputs appear in both:
   - The new `.shopify/bundle/<ext_id>/dist` directory
   - The `dist` folder inside the extension directory.
3. Confirm that source maps are accessible in both locations
4. Test that hot reloading continues to work as expected and both outputs are updated.

### Checklist

- [ ] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [ ] I've considered possible [documentation](https://shopify.dev) changes
  • Loading branch information
isaacroldan authored Nov 21, 2024
2 parents 42b43f7 + 838279a commit 08f8aaf
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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]

Expand All @@ -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)
Expand All @@ -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])

Expand All @@ -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')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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/<ext_id>/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
Expand Down

0 comments on commit 08f8aaf

Please sign in to comment.