Skip to content

Clean up module outputFile calculation and move it to the specifications #6967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alfonso-noriega wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −13
Open

Clean up module outputFile calculation and move it to the specifications #6967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 2 additions & 10 deletions packages/app/src/cli/models/extensions/extension-instance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
}

get outputFileName() {
const mode = this.specification.buildConfig.mode
if (mode === 'copy_files' || mode === 'theme') {
return ''
} else if (mode === 'function') {
return 'index.wasm'
} else {
return `${this.handle}.js`
}
return basename(this.specification.getOutputFileName?.(this) ?? '')
Copy link
Contributor

@ryancbahan ryancbahan Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do all extension instances always have a build/output file name? extension-instance feels very overloaded today, and I worry we're overloading it further with things that might be optional properties.

Copy link
Contributor

@isaacroldan isaacroldan Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the change in this PR actually cleans this. Move the optional implementation to the specification, so each module can decide if it has an outputFileName or not.

This function already existed in extension-instance, but isntead of hardcoded if/else we move the ownership to each specification

}

constructor(options: {
Expand Down Expand Up @@ -408,8 +401,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi

getOutputPathForDirectory(directory: string, outputId?: string) {
const id = this.getOutputFolderId(outputId)
const outputFile = this.outputFileName === '' ? '' : joinPath('dist', this.outputFileName)
return joinPath(directory, id, outputFile)
return joinPath(directory, id, this.specification.getOutputFileName?.(this) ?? '')
}

get singleTarget() {
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/models/extensions/specification.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
buildConfig: BuildConfig
dependency?: string
graphQLType?: string
getOutputFileName?: (extension: ExtensionInstance<TConfiguration>) => string
getBundleExtensionStdinContent?: (config: TConfiguration) => {main: string; assets?: Asset[]}
deployConfig?: (
config: TConfiguration,
Expand Down
5 changes: 5 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/checkout_post_purchase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import {ExtensionInstance} from '../extension-instance.js'
import {BaseSchema, MetafieldSchema} from '../schemas.js'
import {createExtensionSpecification} from '../specification.js'
import {joinPath} from '@shopify/cli-kit/node/path'
import {zod} from '@shopify/cli-kit/node/schema'

const dependency = '@shopify/post-purchase-ui-extensions'

type CheckoutPostPurchaseConfigType = zod.infer<typeof CheckoutPostPurchaseSchema>
const CheckoutPostPurchaseSchema = BaseSchema.extend({
metafields: zod.array(MetafieldSchema).optional(),
})
Expand All @@ -15,6 +18,8 @@ const checkoutPostPurchaseSpec = createExtensionSpecification({
schema: CheckoutPostPurchaseSchema,
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<CheckoutPostPurchaseConfigType>) =>
joinPath('dist', `${extension.handle}.js`),
deployConfig: async (config, _) => {
return {metafields: config.metafields ?? []}
},
Expand Down
4 changes: 4 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema, MetafieldSchema} from '../schemas.js'
import {loadLocalesConfig} from '../../../utilities/extensions/locales-configuration.js'
import {ExtensionInstance} from '../extension-instance.js'
import {zod} from '@shopify/cli-kit/node/schema'
import {joinPath} from '@shopify/cli-kit/node/path'

const dependency = '@shopify/checkout-ui-extensions'

type CheckoutConfigType = zod.infer<typeof CheckoutSchema>
const CheckoutSchema = BaseSchema.extend({
name: zod.string(),
extension_points: zod.array(zod.string()).optional(),
Expand All @@ -22,6 +25,7 @@ const checkoutSpec = createExtensionSpecification({
schema: CheckoutSchema,
appModuleFeatures: (_) => ['ui_preview', 'cart_url', 'esbuild', 'single_js_entry_path', 'generates_source_maps'],
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<CheckoutConfigType>) => joinPath('dist', `${extension.handle}.js`),
Copy link

@binks-code-reviewer binks-code-reviewer bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dist/dist/... path regression for UI extensions due to double dist/ prefix

getOutputFileName returns joinPath('dist', ${extension.handle}.js), which already includes dist/, while ExtensionInstance builds outputPath as joinPath(this.directory, 'dist', this.outputFileName ?? ''), producing .../<ext>/dist/dist/<handle>.js. This breaks expected artifact locations and later steps (upload/deploy/preview) that assume dist/<handle>.js.

deployConfig: async (config, directory) => {
return {
extension_points: config.extension_points,
Expand Down
2 changes: 2 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/function.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema} from '../schemas.js'
import {loadLocalesConfig} from '../../../utilities/extensions/locales-configuration.js'
import {ExtensionInstance} from '../extension-instance.js'
import {zod} from '@shopify/cli-kit/node/schema'
import {joinPath} from '@shopify/cli-kit/node/path'
import {fileExists, readFile} from '@shopify/cli-kit/node/fs'
Expand Down Expand Up @@ -88,6 +89,7 @@ const functionSpec = createExtensionSpecification({
schema: FunctionExtensionSchema,
appModuleFeatures: (_) => ['function'],
buildConfig: {mode: 'function'},
getOutputFileName: (_extension: ExtensionInstance<FunctionConfigType>) => joinPath('dist', 'index.wasm'),
Copy link

@binks-code-reviewer binks-code-reviewer bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function spec returns dist/index.wasm, conflicting with existing function output path logic

Function getOutputFileName returns joinPath('dist', 'index.wasm'), but ExtensionInstance already has function-specific logic that expects the build path to include dist/index.wasm via config/defaultPath, and other code paths use this.outputFileName ?? '' directly. This can lead to inconsistent read/write locations and missing wasm artifacts depending on which path computation is used.

deployConfig: async (config, directory, apiKey) => {
let inputQuery: string | undefined
const moduleId = randomUUID()
Expand Down
8 changes: 7 additions & 1 deletion packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import {getDependencyVersion} from '../../app/app.js'
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema} from '../schemas.js'
import {ExtensionInstance} from '../extension-instance.js'
import {BugError} from '@shopify/cli-kit/node/error'
import {zod} from '@shopify/cli-kit/node/schema'
import {joinPath} from '@shopify/cli-kit/node/path'

const dependency = '@shopify/retail-ui-extensions'

type PosUIConfigType = zod.infer<typeof PosUISchema>
const PosUISchema = BaseSchema.extend({name: zod.string()})

const posUISpec = createExtensionSpecification({
identifier: 'pos_ui_extension',
dependency,
schema: BaseSchema.extend({name: zod.string()}),
schema: PosUISchema,
appModuleFeatures: (_) => ['ui_preview', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<PosUIConfigType>) => joinPath('dist', `${extension.handle}.js`),
deployConfig: async (config, directory) => {
const result = await getDependencyVersion(dependency, directory)
if (result === 'not_found') throw new BugError(`Dependency ${dependency} not found`)
Expand Down
7 changes: 7 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/product_subscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import {getDependencyVersion} from '../../app/app.js'
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema} from '../schemas.js'
import {ExtensionInstance} from '../extension-instance.js'
import {BugError} from '@shopify/cli-kit/node/error'
import {zod} from '@shopify/cli-kit/node/schema'
import {joinPath} from '@shopify/cli-kit/node/path'

const dependency = '@shopify/admin-ui-extensions'

type ProductSubscriptionConfigType = zod.infer<typeof BaseSchema>

const productSubscriptionSpec = createExtensionSpecification({
identifier: 'product_subscription',
additionalIdentifiers: ['subscription_management'],
Expand All @@ -13,6 +18,8 @@ const productSubscriptionSpec = createExtensionSpecification({
schema: BaseSchema,
appModuleFeatures: (_) => ['ui_preview', 'esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<ProductSubscriptionConfigType>) =>
joinPath('dist', `${extension.handle}.js`),
deployConfig: async (_, directory) => {
const result = await getDependencyVersion(dependency, directory)
if (result === 'not_found') throw new BugError(`Dependency ${dependency} not found`)
Expand Down
5 changes: 5 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/tax_calculation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema, MetafieldSchema} from '../schemas.js'
import {ExtensionInstance} from '../extension-instance.js'
import {zod} from '@shopify/cli-kit/node/schema'
import {joinPath} from '@shopify/cli-kit/node/path'

const CartLinePropertySchema = zod.object({
key: zod.string(),
})

type TaxCalculationsConfigType = zod.infer<typeof TaxCalculationsSchema>
const TaxCalculationsSchema = BaseSchema.extend({
production_api_base_url: zod.string(),
benchmark_api_base_url: zod.string().optional(),
Expand All @@ -29,6 +32,8 @@ const spec = createExtensionSpecification({
schema: TaxCalculationsSchema,
appModuleFeatures: (_) => [],
buildConfig: {mode: 'tax_calculation'},
getOutputFileName: (extension: ExtensionInstance<TaxCalculationsConfigType>) =>
joinPath('dist', `${extension.handle}.js`),
deployConfig: async (config, _) => {
return {
production_api_base_url: config.production_api_base_url,
Expand Down
3 changes: 3 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/ui_extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface BuildManifest {

const missingExtensionPointsMessage = 'No extension targets defined, add a `targeting` field to your configuration'

type UIExtensionConfigType = zod.infer<typeof UIExtensionSchema>
export const UIExtensionSchema = BaseSchema.extend({
name: zod.string(),
type: zod.literal('ui_extension'),
Expand Down Expand Up @@ -102,6 +103,8 @@ const uiExtensionSpec = createExtensionSpecification({
dependency,
schema: UIExtensionSchema,
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<UIExtensionConfigType>) =>
joinPath('dist', `${extension.handle}.js`),
appModuleFeatures: (config) => {
const basic: ExtensionFeature[] = ['ui_preview', 'esbuild', 'generates_source_maps']
const needsCart =
Expand Down
4 changes: 4 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import {createExtensionSpecification} from '../specification.js'
import {BaseSchema} from '../schemas.js'
import {ExtensionInstance} from '../extension-instance.js'
import {zod} from '@shopify/cli-kit/node/schema'
import {AbortError} from '@shopify/cli-kit/node/error'
import {fileSize} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'

const kilobytes = 1024
const BUNDLE_SIZE_LIMIT_KB = 128
const BUNDLE_SIZE_LIMIT = BUNDLE_SIZE_LIMIT_KB * kilobytes

const dependency = '@shopify/web-pixels-extension'

type WebPixelConfigType = zod.infer<typeof WebPixelSchema>
const WebPixelSchema = BaseSchema.extend({
runtime_context: zod.string(),
version: zod.string().optional(),
Expand All @@ -32,6 +35,7 @@ const webPixelSpec = createExtensionSpecification({
schema: WebPixelSchema,
appModuleFeatures: (_) => ['esbuild', 'single_js_entry_path'],
buildConfig: {mode: 'ui'},
getOutputFileName: (extension: ExtensionInstance<WebPixelConfigType>) => joinPath('dist', `${extension.handle}.js`),
deployConfig: async (config, _) => {
return {
runtime_context: config.runtime_context,
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/dev/extension/server/middlewares.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {getHTML} from '../templates.js'
import {getWebSocketUrl} from '../../extension.js'
import {fileExists, isDirectory, readFile, findPathUp} from '@shopify/cli-kit/node/fs'
import {IncomingMessage, ServerResponse, sendRedirect, send} from 'h3'
import {joinPath, extname, moduleDirectory} from '@shopify/cli-kit/node/path'
import {joinPath, dirname, extname, moduleDirectory} from '@shopify/cli-kit/node/path'
import {outputDebug} from '@shopify/cli-kit/node/output'

export function corsMiddleware(_request: IncomingMessage, response: ServerResponse, next: (err?: Error) => unknown) {
Expand Down Expand Up @@ -90,7 +90,7 @@ export function getExtensionAssetMiddleware({devOptions, getExtensions}: GetExte
const bundlePath = devOptions.appWatcher.buildOutputPath
const extensionOutputPath = extension.getOutputPathForDirectory(bundlePath)

const buildDirectory = extensionOutputPath.replace(extension.outputFileName, '')
const buildDirectory = dirname(extensionOutputPath)

return fileServerMiddleware(request, response, next, {
filePath: joinPath(buildDirectory, assetPath),
Expand Down
Loading