Skip to content

Commit

Permalink
Use AppWatcher events for extension drafts (#4889)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Improves the extension development workflow by integrating with the app event watcher system instead of maintaining separate file watchers for each extension.

### WHAT is this pull request doing?

Refactors the extension draft update process to:
- Remove individual extension file watchers
- Integrate with the centralized app event watcher system
- Update extension drafts only when build results are successful

### How to test your changes?

1. Run `dev` against partners with a project containing multiple extensions
2. Verify that extension changes are detected and drafts are updated
3. Confirm that failed builds don't trigger draft updates

### Measuring impact

- [x] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

### Checklist

- [x] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [x] I've considered possible [documentation](https://shopify.dev) changes
  • Loading branch information
isaacroldan authored Nov 21, 2024
2 parents 7e44ab7 + 6d8106d commit 42b43f7
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 64 deletions.
90 changes: 31 additions & 59 deletions packages/app/src/cli/services/dev/processes/draftable-extension.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import {BaseProcess, DevProcessFunction} from './types.js'
import {updateExtensionDraft} from '../update-extension.js'
import {setupExtensionWatcher} from '../extension/bundler.js'
import {ExtensionInstance} from '../../../models/extensions/extension-instance.js'
import {AppInterface} from '../../../models/app/app.js'
import {PartnersAppForIdentifierMatching, ensureDeploymentIdsPresence} from '../../context/identifiers.js'
import {getAppIdentifiers} from '../../../models/app/identifiers.js'
import {installJavy} from '../../function/build.js'
import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js'
import {AppEvent, AppEventWatcher, EventType} from '../app-events/app-event-watcher.js'
import {performActionWithRetryAfterRecovery} from '@shopify/cli-kit/common/retry'
import {AbortError} from '@shopify/cli-kit/node/error'
import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components'
import {outputWarn} from '@shopify/cli-kit/node/output'

interface DraftableExtensionOptions {
extensions: ExtensionInstance[]
Expand All @@ -19,15 +18,16 @@ interface DraftableExtensionOptions {
remoteExtensionIds: {[key: string]: string}
proxyUrl: string
localApp: AppInterface
appWatcher: AppEventWatcher
}

export interface DraftableExtensionProcess extends BaseProcess<DraftableExtensionOptions> {
type: 'draftable-extension'
}

export const pushUpdatesForDraftableExtensions: DevProcessFunction<DraftableExtensionOptions> = async (
{stderr, stdout, abortSignal: signal},
{extensions, developerPlatformClient, apiKey, remoteExtensionIds: remoteExtensions, proxyUrl, localApp: app},
{stderr, stdout},
{developerPlatformClient, apiKey, remoteExtensionIds: remoteExtensions, localApp: app, appWatcher},
) => {
// Force the download of the javy binary in advance to avoid later problems,
// as it might be done multiple times in parallel. https://github.com/Shopify/cli/issues/2877
Expand All @@ -37,63 +37,35 @@ export const pushUpdatesForDraftableExtensions: DevProcessFunction<DraftableExte
await developerPlatformClient.refreshToken()
}

await Promise.all(
extensions.map(async (extension) => {
const handleAppEvent = (event: AppEvent) => {
const extensionEvents = event.extensionEvents
.filter((ev) => ev.type === EventType.Updated)
.filter((ev) => ev.buildResult?.status === 'ok')

for (const extensionEvent of extensionEvents) {
const extension = extensionEvent.extension
const registrationId = remoteExtensions[extension.localIdentifier]
if (!registrationId) throw new AbortError(`Extension ${extension.localIdentifier} not found on remote app.`)
return useConcurrentOutputContext({outputPrefix: extension.outputPrefix}, async () => {
await extension.build({
app,
stdout,
stderr,
useTasks: false,
signal,
environment: 'development',
})
const registrationId = remoteExtensions[extension.localIdentifier]
if (!registrationId) throw new AbortError(`Extension ${extension.localIdentifier} not found on remote app.`)
// Initial draft update for each extension
await updateExtensionDraft({
extension,
developerPlatformClient,
apiKey,
registrationId,
stdout,
stderr,
appConfiguration: app.configuration,
})
// Watch for changes
return setupExtensionWatcher({
extension,
app,
url: proxyUrl,
stdout,
stderr,
signal,
onChange: async () => {
// At this point the extension has already been built and is ready to be updated
return performActionWithRetryAfterRecovery(
async () =>
updateExtensionDraft({
extension,
developerPlatformClient,
apiKey,
registrationId,
stdout,
stderr,
appConfiguration: app.configuration,
}),
refreshToken,
)
},
onReloadAndBuildError: async (error) => {
const draftUpdateErrorMessage = extension.draftMessages.errorMessage
if (draftUpdateErrorMessage) {
outputWarn(`${draftUpdateErrorMessage}: ${error.message}`, stdout)
}
},
})
return performActionWithRetryAfterRecovery(
async () =>
updateExtensionDraft({
extension,
developerPlatformClient,
apiKey,
registrationId,
stdout,
stderr,
appConfiguration: app.configuration,
bundlePath: appWatcher.buildOutputPath,
}),
refreshToken,
)
})
}),
)
}
}

appWatcher.onEvent(handleAppEvent).onStart(handleAppEvent)
}

export async function setupDraftableExtensionsProcess({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export async function setupDevProcesses({
apiKey,
developerPlatformClient,
proxyUrl: network.proxyUrl,
appWatcher,
}),
await setupPreviewThemeAppExtensionsProcess({
remoteApp,
Expand Down
16 changes: 12 additions & 4 deletions packages/app/src/cli/services/dev/update-extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ describe('updateExtensionDraft()', () => {
directory: tmpDir,
})

await mkdir(joinPath(tmpDir, 'dist'))
await writeFile(mockExtension.outputPath, 'test content')
await mkdir(joinPath(tmpDir, 'mock-handle', 'dist'))
const outputPath = mockExtension.getOutputPathForDirectory(tmpDir)
await writeFile(outputPath, 'test content')

await updateExtensionDraft({
extension: mockExtension,
Expand All @@ -62,6 +63,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: tmpDir,
})

expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({
Expand Down Expand Up @@ -93,6 +95,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: 'dir',
})

expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({
Expand Down Expand Up @@ -136,6 +139,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: tmpDir,
})

expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({
Expand Down Expand Up @@ -173,6 +177,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: tmpDir,
})

expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({
Expand Down Expand Up @@ -211,6 +216,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: tmpDir,
})

expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({
Expand Down Expand Up @@ -253,8 +259,9 @@ describe('updateExtensionDraft()', () => {
type: 'web_pixel_extension',
})

await mkdir(joinPath(tmpDir, 'dist'))
await writeFile(mockExtension.outputPath, 'test content')
await mkdir(joinPath(tmpDir, mockExtension.handle, 'dist'))
const outputPath = mockExtension.getOutputPathForDirectory(tmpDir)
await writeFile(outputPath, 'test content')

await updateExtensionDraft({
extension: mockExtension,
Expand All @@ -264,6 +271,7 @@ describe('updateExtensionDraft()', () => {
stdout,
stderr,
appConfiguration: placeholderAppConfiguration,
bundlePath: tmpDir,
})

expect(stderr.write).toHaveBeenCalledWith('Error while updating drafts: Error1, Error2')
Expand Down
5 changes: 4 additions & 1 deletion packages/app/src/cli/services/dev/update-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface UpdateExtensionDraftOptions {
stdout: Writable
stderr: Writable
appConfiguration: AppConfigurationWithoutPath
bundlePath: string
}

export async function updateExtensionDraft({
Expand All @@ -34,10 +35,12 @@ export async function updateExtensionDraft({
stdout,
stderr,
appConfiguration,
bundlePath,
}: UpdateExtensionDraftOptions) {
let encodedFile: string | undefined
if (extension.features.includes('esbuild')) {
const content = await readFile(extension.outputPath)
const outputPath = extension.getOutputPathForDirectory(bundlePath)
const content = await readFile(outputPath)
if (!content) return
encodedFile = Buffer.from(content).toString('base64')
}
Expand Down

0 comments on commit 42b43f7

Please sign in to comment.