Skip to content

Commit 8cac9b8

Browse files
vividvioletvividviolet
committed
Fix Dev Server to allow serving static assets from a shared folder outside of the extension directory
1 parent d5b7839 commit 8cac9b8

10 files changed

Lines changed: 537 additions & 118 deletions

File tree

‎packages/app/src/cli/services/build/steps/include-assets-step.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import {generateManifestFile} from './include-assets/generate-manifest.js'
1+
import {generateManifestFile, resolveOutputDir} from './include-assets/generate-manifest.js'
22
import {copyByPattern} from './include-assets/copy-by-pattern.js'
33
import {copySourceEntry} from './include-assets/copy-source-entry.js'
44
import {copyConfigKeyEntry} from './include-assets/copy-config-key-entry.js'
5-
import {joinPath, dirname, extname, sanitizeRelativePath} from '@shopify/cli-kit/node/path'
5+
import {joinPath, sanitizeRelativePath} from '@shopify/cli-kit/node/path'
66
import {z} from 'zod'
77
import type {LifecycleStep, BuildContext} from '../client-steps.js'
88

@@ -123,9 +123,7 @@ export async function executeIncludeAssetsStep(
123123
): Promise<{filesCopied: number}> {
124124
const config = IncludeAssetsConfigSchema.parse(step.config)
125125
const {extension, options} = context
126-
// When outputPath is a file (e.g. index.js, index.wasm), the output directory is its
127-
// parent. When outputPath has no extension, it IS the output directory.
128-
const outputDir = extname(extension.outputPath) ? dirname(extension.outputPath) : extension.outputPath
126+
const outputDir = resolveOutputDir(extension.outputPath)
129127

130128
const aggregatedPathMap = new Map<string, string | string[]>()
131129
// Track basenames written across all configKey entries in this build to detect

‎packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import {fileExists, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs'
44
import {outputDebug} from '@shopify/cli-kit/node/output'
55
import type {BuildContext} from '../../client-steps.js'
66

7+
/**
8+
* Resolves the output directory from an extension's outputPath.
9+
* When outputPath is a file (has extension), uses dirname. Otherwise uses outputPath directly.
10+
*/
11+
export function resolveOutputDir(outputPath: string): string {
12+
return extname(outputPath) ? dirname(outputPath) : outputPath
13+
}
14+
715
interface ConfigKeyManifestEntry {
816
anchor?: string | undefined
917
groupBy?: string | undefined
@@ -121,12 +129,7 @@ export async function createOrUpdateManifestFile(
121129
context: BuildContext,
122130
entries: {[key: string]: unknown},
123131
): Promise<void> {
124-
const outputPath = context.extension.outputPath
125-
/**
126-
* Resolves the output directory from an extension's outputPath.
127-
* When outputPath is a file (has extension), uses dirname. Otherwise uses outputPath directly.
128-
*/
129-
const outputDir = extname(outputPath) ? dirname(outputPath) : outputPath
132+
const outputDir = resolveOutputDir(context.extension.outputPath)
130133

131134
const manifestPath = joinPath(outputDir, 'manifest.json')
132135

‎packages/app/src/cli/services/dev/extension.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ describe('devUIExtensions()', () => {
6161
expect(store.ExtensionsPayloadStore).toHaveBeenCalledWith(
6262
{mock: 'payload'},
6363
{...options, websocketURL: 'wss://mock.url/extensions'},
64+
expect.any(Map),
6465
)
6566
})
6667

‎packages/app/src/cli/services/dev/extension.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ export async function devUIExtensions(options: ExtensionDevOptions): Promise<voi
125125
// affecting the original `options` object and we only need to care about `payloadOptions` in this function.
126126

127127
const bundlePath = payloadOptions.appWatcher.buildOutputPath
128-
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadOptions, bundlePath)
129-
const payloadStore = new ExtensionsPayloadStore(payloadStoreRawPayload, payloadOptions)
128+
const assetResolvers = new Map()
129+
const payloadStoreRawPayload = await getExtensionsPayloadStoreRawPayload(payloadOptions, bundlePath, assetResolvers)
130+
const payloadStore = new ExtensionsPayloadStore(payloadStoreRawPayload, payloadOptions, assetResolvers)
130131
let extensions = payloadOptions.extensions.filter((ext) => ext.isPreviewable)
131132

132133
const getExtensions = () => {

‎packages/app/src/cli/services/dev/extension/payload.test.ts‎

Lines changed: 94 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,12 @@ describe('getUIExtensionPayload', () => {
192192
assets: {
193193
tools: {
194194
name: 'tools',
195-
url: 'http://tunnel-url.com/extensions/devUUID/assets/tools.json',
195+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/tools',
196196
lastUpdated: expect.any(Number),
197197
},
198198
instructions: {
199199
name: 'instructions',
200-
url: 'http://tunnel-url.com/extensions/devUUID/assets/instructions.md',
200+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/instructions',
201201
lastUpdated: expect.any(Number),
202202
},
203203
},
@@ -241,12 +241,12 @@ describe('getUIExtensionPayload', () => {
241241
assets: {
242242
main: {
243243
name: 'main',
244-
url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension.js',
244+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/main',
245245
lastUpdated: expect.any(Number),
246246
},
247247
should_render: {
248248
name: 'should_render',
249-
url: 'http://tunnel-url.com/extensions/devUUID/assets/test-ui-extension-conditions.js',
249+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/should_render',
250250
lastUpdated: expect.any(Number),
251251
},
252252
},
@@ -295,12 +295,12 @@ describe('getUIExtensionPayload', () => {
295295
assets: {
296296
main: {
297297
name: 'main',
298-
url: 'http://tunnel-url.com/extensions/devUUID/assets/dist/test-ui-extension.js',
298+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/main',
299299
lastUpdated: expect.any(Number),
300300
},
301301
should_render: {
302302
name: 'should_render',
303-
url: 'http://tunnel-url.com/extensions/devUUID/assets/dist/test-ui-extension-conditions.js',
303+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/should_render',
304304
lastUpdated: expect.any(Number),
305305
},
306306
},
@@ -309,6 +309,91 @@ describe('getUIExtensionPayload', () => {
309309
})
310310
})
311311

312+
test('emits a distinct URL per extension point even when built assets share a filepath', async () => {
313+
await inTemporaryDirectory(async (tmpDir) => {
314+
const uiExtension = await testUIExtension({
315+
directory: tmpDir,
316+
configuration: {
317+
name: 'test-ui-extension',
318+
type: 'ui_extension',
319+
extension_points: [
320+
{target: 'TARGET_A', module: './src/ExtensionPointA.js'},
321+
{target: 'TARGET_B', module: './src/ExtensionPointB.js'},
322+
],
323+
},
324+
devUUID: 'devUUID',
325+
})
326+
327+
await setupBuildOutput(
328+
uiExtension,
329+
tmpDir,
330+
{
331+
TARGET_A: {main: 'dist/main.js'},
332+
TARGET_B: {main: 'dist/main.js'},
333+
},
334+
{},
335+
)
336+
337+
const resolver = new Map<string, string>()
338+
const got = await getUIExtensionPayload(
339+
uiExtension,
340+
tmpDir,
341+
{
342+
...createMockOptions(tmpDir, [uiExtension]),
343+
currentDevelopmentPayload: {hidden: true, status: 'success'},
344+
},
345+
resolver,
346+
)
347+
348+
expect(got.extensionPoints).toMatchObject([
349+
{
350+
target: 'TARGET_A',
351+
assets: {
352+
main: {name: 'main', url: 'http://tunnel-url.com/extensions/devUUID/assets/TARGET_A/main'},
353+
},
354+
},
355+
{
356+
target: 'TARGET_B',
357+
assets: {
358+
main: {name: 'main', url: 'http://tunnel-url.com/extensions/devUUID/assets/TARGET_B/main'},
359+
},
360+
},
361+
])
362+
expect(resolver.get('TARGET_A/main')).toBe('dist/main.js')
363+
expect(resolver.get('TARGET_B/main')).toBe('dist/main.js')
364+
})
365+
})
366+
367+
test('clears stale resolver entries on each payload regeneration', async () => {
368+
await inTemporaryDirectory(async (tmpDir) => {
369+
const uiExtension = await testUIExtension({
370+
directory: tmpDir,
371+
configuration: {
372+
name: 'test-ui-extension',
373+
type: 'ui_extension',
374+
extension_points: [{target: 'CUSTOM_EXTENSION_POINT', module: './src/ExtensionPointA.js'}],
375+
},
376+
devUUID: 'devUUID',
377+
})
378+
379+
await setupBuildOutput(uiExtension, tmpDir, {CUSTOM_EXTENSION_POINT: {main: 'dist/main.js'}}, {})
380+
381+
const resolver = new Map<string, string>([['STALE_TARGET/main', 'stale.js']])
382+
await getUIExtensionPayload(
383+
uiExtension,
384+
tmpDir,
385+
{
386+
...createMockOptions(tmpDir, [uiExtension]),
387+
currentDevelopmentPayload: {hidden: true, status: 'success'},
388+
},
389+
resolver,
390+
)
391+
392+
expect(resolver.has('STALE_TARGET/main')).toBe(false)
393+
expect(resolver.get('CUSTOM_EXTENSION_POINT/main')).toBe('dist/main.js')
394+
})
395+
})
396+
312397
test('maps intents from manifest.json to asset payloads', async () => {
313398
await inTemporaryDirectory(async (tmpDir) => {
314399
const uiExtension = await testUIExtension({
@@ -351,7 +436,7 @@ describe('getUIExtensionPayload', () => {
351436
action: 'create',
352437
schema: {
353438
name: 'schema',
354-
url: 'http://tunnel-url.com/extensions/devUUID/assets/intents/create-schema.json',
439+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/intents/0/schema',
355440
lastUpdated: expect.any(Number),
356441
},
357442
},
@@ -360,7 +445,7 @@ describe('getUIExtensionPayload', () => {
360445
action: 'update',
361446
schema: {
362447
name: 'schema',
363-
url: 'http://tunnel-url.com/extensions/devUUID/assets/intents/update-schema.json',
448+
url: 'http://tunnel-url.com/extensions/devUUID/assets/CUSTOM_EXTENSION_POINT/intents/1/schema',
364449
lastUpdated: expect.any(Number),
365450
},
366451
},
@@ -461,7 +546,7 @@ describe('getUIExtensionPayload', () => {
461546
assets: {
462547
tools: {
463548
name: 'tools',
464-
url: 'http://tunnel-url.com/extensions/devUUID/assets/tools.json',
549+
url: 'http://tunnel-url.com/extensions/devUUID/assets/admin.app.intent.link/tools',
465550
lastUpdated: expect.any(Number),
466551
},
467552
},

0 commit comments

Comments
 (0)