Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
return this.specification.devSessionWatchConfig(this)
}

return this.specification.experience === 'configuration' ? {paths: []} : undefined
return this.isAppConfigExtension ? {paths: []} : undefined
}

async watchConfigurationPaths() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ const AdminSchema = zod.object({
admin: zod
.object({
static_root: zod.string().optional(),
allowed_domains: zod.array(zod.string()).optional(),
})
.optional(),
})

type AdminConfigType = zod.infer<typeof AdminSchema> & BaseConfigType
export type AdminConfigType = zod.infer<typeof AdminSchema> & BaseConfigType

const adminSpecificationSpec = createExtensionSpecification<AdminConfigType>({
identifier: 'admin',
Expand All @@ -33,6 +34,8 @@ const adminSpecificationSpec = createExtensionSpecification<AdminConfigType>({
admin: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
static_root: (remoteContent as any).admin.static_root,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
allowed_domains: (remoteContent as any).admin.allowed_domains,
},
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ describe('executeIncludeAssetsStep', () => {
)
})

test('throws when manifest.json already exists in the output directory', async () => {
test('overwrites manifest.json when it already exists in the output directory', async () => {
// Given — a prior inclusion already copied a manifest.json to the output dir
const contextWithConfig = {
...mockContext,
Expand All @@ -1056,8 +1056,7 @@ describe('executeIncludeAssetsStep', () => {
} as unknown as ExtensionInstance,
}

// Source files exist; output manifest.json already exists (simulating conflict);
// candidate output paths for tools.json are free so copyConfigKeyEntry succeeds.
// Source files exist; output manifest.json already exists
vi.mocked(fs.fileExists).mockImplementation(async (path) => {
const pathStr = String(path)
return pathStr === '/test/output/manifest.json' || pathStr.startsWith('/test/extension/')
Expand All @@ -1081,10 +1080,9 @@ describe('executeIncludeAssetsStep', () => {
},
}

// When / Then — throws rather than silently overwriting
await expect(executeIncludeAssetsStep(step, contextWithConfig)).rejects.toThrow(
`Can't write manifest.json: a file already exists at '/test/output/manifest.json'`,
)
// When / Then — overwrites existing manifest.json
await expect(executeIncludeAssetsStep(step, contextWithConfig)).resolves.not.toThrow()
expect(fs.writeFile).toHaveBeenCalledWith('/test/output/manifest.json', expect.any(String))
})

test('writes an empty manifest when anchor resolves to a non-array value', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {getNestedValue, tokenizePath} from './copy-config-key-entry.js'
import {joinPath} from '@shopify/cli-kit/node/path'
import {fileExists, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
import {mkdir, writeFile} from '@shopify/cli-kit/node/fs'
import {outputDebug} from '@shopify/cli-kit/node/output'
import type {BuildContext} from '../../client-steps.js'

Expand All @@ -20,7 +20,7 @@ interface ConfigKeyManifestEntry {
* 3. Build root-level entries.
* 4. Build grouped entries (anchor/groupBy logic) with path strings resolved
* via `resolveManifestPaths` using the copy-tracked `pathMap`.
* 5. Write `outputDir/manifest.json`; throw if the file already exists.
* 5. Write `outputDir/manifest.json`, overwriting any existing file.
*
* @param pathMap - Map from raw config path values to their output-relative
* paths, as recorded during the copy phase by `copyConfigKeyEntry`.
Expand Down Expand Up @@ -113,12 +113,6 @@ export async function generateManifestFile(
}

const manifestPath = joinPath(outputDir, 'manifest.json')
if (await fileExists(manifestPath)) {
throw new Error(
`Can't write manifest.json: a file already exists at '${manifestPath}'. ` +
`Remove or rename the conflicting inclusion to avoid overwriting the generated manifest.`,
)
}
await mkdir(outputDir)
await writeFile(manifestPath, JSON.stringify(manifest, null, 2))
outputDebug(`Generated manifest.json in ${outputDir}\n`, options.stdout)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export class FileWatcher {
private getAllWatchedFiles(): string[] {
this.extensionWatchedFiles.clear()

const extensionResults = this.app.nonConfigExtensions.map((extension) => ({
const extensionResults = this.app.realExtensions.map((extension) => ({
extension,
watchedFiles: extension.watchedFiles(),
}))
Comment thread
isaacroldan marked this conversation as resolved.
Expand Down
14 changes: 11 additions & 3 deletions packages/app/src/cli/services/dev/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,14 @@ describe('devUIExtensions()', () => {

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
vi.spyOn(store, 'ExtensionsPayloadStore').mockImplementation(() => ({mock: 'payload-store'}))
vi.spyOn(store, 'ExtensionsPayloadStore').mockImplementation(
() =>
({
mock: 'payload-store',
updateAdminConfigFromExtensionEvents: vi.fn(),
getAppAssets: vi.fn(),
}) as unknown as store.ExtensionsPayloadStore,
)
vi.spyOn(server, 'setupHTTPServer').mockReturnValue({
mock: 'http-server',
close: serverCloseSpy,
Expand Down Expand Up @@ -67,8 +74,9 @@ describe('devUIExtensions()', () => {
// THEN
expect(server.setupHTTPServer).toHaveBeenCalledWith({
devOptions: {...options, websocketURL: 'wss://mock.url/extensions'},
payloadStore: {mock: 'payload-store'},
payloadStore: expect.objectContaining({mock: 'payload-store'}),
getExtensions: expect.any(Function),
getAppAssets: expect.any(Function),
})
})

Expand All @@ -94,7 +102,7 @@ describe('devUIExtensions()', () => {
expect(websocket.setupWebsocketConnection).toHaveBeenCalledWith({
...options,
httpServer: expect.objectContaining({mock: 'http-server'}),
payloadStore: {mock: 'payload-store'},
payloadStore: expect.objectContaining({mock: 'payload-store'}),
websocketURL: 'wss://mock.url/extensions',
})
})
Expand Down
18 changes: 15 additions & 3 deletions packages/app/src/cli/services/dev/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ export interface ExtensionDevOptions {
buildDirectory?: string

/**
* The extension to be built.
* All real extensions in the app, including non-previewable ones (e.g., admin config).
* Previewable extensions are filtered internally for the UI payload.
*/
extensions: ExtensionInstance[]

Expand Down Expand Up @@ -126,14 +127,20 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
const bundlePath = payloadOptions.appWatcher.buildOutputPath
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadOptions, bundlePath)
const payloadStore = new ExtensionsPayloadStore(payloadStoreRawPayload, payloadOptions)
let extensions = payloadOptions.extensions
let extensions = payloadOptions.extensions.filter((ext) => ext.isPreviewable)

const getExtensions = () => {
return extensions
}

outputDebug(`Setting up the UI extensions HTTP server...`, payloadOptions.stdout)
const httpServer = setupHTTPServer({devOptions: payloadOptions, payloadStore, getExtensions})
const getAppAssets = () => payloadStore.getAppAssets()
const httpServer = setupHTTPServer({
devOptions: payloadOptions,
payloadStore,
getExtensions,
getAppAssets,
})

outputDebug(`Setting up the UI extensions Websocket server...`, payloadOptions.stdout)
const websocketConnection = setupWebsocketConnection({...payloadOptions, httpServer, payloadStore})
Expand All @@ -144,6 +151,11 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
extensions = app.allExtensions.filter((ext) => ext.isPreviewable)
}

// Exception for AdminConfig: it's a extension that needs its config extracted at the app level
// for the payloed. No other exceptions should be added, if this pattern is needed again we should
// generalize it.
payloadStore.updateAdminConfigFromExtensionEvents(extensionEvents)

for (const event of extensionEvents) {
if (!event.extension.isPreviewable) continue
const status = event.buildResult?.status === 'ok' ? 'success' : 'error'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ interface ExtensionsPayloadInterface {
url: string
mobileUrl: string
title: string
allowedDomains?: string[]
assets?: {
[key: string]: {
url: string
lastUpdated: number
}
}
}
appId?: string
store: string
Expand Down
Loading
Loading