Merge pull request #6484 from Shopify/fix-default-function-output-path

Fix default output build path for functions

Author: gonzaloriestra

packages/app/src/cli/models/extensions/extension-instance.test.ts

@@ -551,4 +551,59 @@ describe('draftMessages', async () => {
       expect(extensionInstance.uid).toBe('orders/delete::123::https://my-app.com/webhooks')
     })
   })
+
+  describe('outputPath for function extensions', async () => {
+    test('uses default path when build is undefined', async () => {
+      // Given
+      const config = {
+        name: 'foo',
+        type: 'function',
+        api_version: '2023-07',
+        configuration_ui: true,
+        // build is intentionally omitted to test undefined case
+      } as FunctionConfigType
+
+      const extensionInstance = await testFunctionExtension({
+        config,
+        dir: 'test-function',
+      })
+
+      // Then
+      expect(extensionInstance.outputPath).toBe(joinPath('test-function', 'dist', 'index.wasm'))
+    })
+
+    test('uses default path when build.path is undefined', async () => {
+      // Given
+      const config = functionConfiguration()
+      config.build = {
+        wasm_opt: true,
+        // path is not defined
+      }
+
+      const extensionInstance = await testFunctionExtension({
+        config,
+        dir: 'test-function',
+      })
+
+      // Then
+      expect(extensionInstance.outputPath).toBe(joinPath('test-function', 'dist', 'index.wasm'))
+    })
+
+    test('uses custom path when build.path is defined', async () => {
+      // Given
+      const config = functionConfiguration()
+      config.build = {
+        wasm_opt: true,
+        path: 'custom/output.wasm',
+      }
+
+      const extensionInstance = await testFunctionExtension({
+        config,
+        dir: 'test-function',
+      })
+
+      // Then
+      expect(extensionInstance.outputPath).toBe(joinPath('test-function', 'custom/output.wasm'))
+    })
+  })
 })

packages/app/src/cli/models/extensions/extension-instance.ts

@@ -169,7 +169,8 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
 
     if (this.isFunctionExtension) {
       const config = this.configuration as unknown as FunctionConfigType
-      this.outputPath = joinPath(this.directory, config.build.path ?? joinPath('dist', 'index.wasm'))
+      const defaultPath = joinPath('dist', 'index.wasm')
+      this.outputPath = joinPath(this.directory, config.build?.path ?? defaultPath)
     }
   }
 
@@ -280,7 +281,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
   // Functions specific properties
   get buildCommand() {
     const config = this.configuration as unknown as FunctionConfigType
-    return config.build.command
+    return config.build?.command
   }
 
   // Paths to be watched in a dev session
@@ -302,7 +303,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
   get watchBuildPaths() {
     if (this.isFunctionExtension) {
       const config = this.configuration as unknown as FunctionConfigType
-      const configuredPaths = config.build.watch ? [config.build.watch].flat() : []
+      const configuredPaths = config.build?.watch ? [config.build.watch].flat() : []
 
       if (!this.isJavaScript && configuredPaths.length === 0) {
         return null

packages/app/src/cli/models/extensions/specifications/function.test.ts

@@ -305,4 +305,27 @@ describe('functionConfiguration', () => {
       expect(got.ui?.ui_extension_handle).toBeUndefined()
     })
   })
+
+  test('accepts configuration without build section', async () => {
+    // Given
+    const configWithoutBuild = {
+      name: 'function',
+      type: 'function',
+      metafields: [],
+      description: 'my function',
+      // build section is omitted
+      configuration_ui: false,
+      api_version: '2022-07',
+    }
+
+    // When
+    const extension = await testFunctionExtension({
+      dir: '/function',
+      config: configWithoutBuild as FunctionConfigType,
+    })
+
+    // Then
+    expect(extension.configuration.build).toBeUndefined()
+    expect(extension.outputPath).toBe(joinPath('/function', 'dist', 'index.wasm'))
+  })
 })

packages/app/src/cli/models/extensions/specifications/function.ts

@@ -18,15 +18,17 @@ interface UI {
 
 export type FunctionConfigType = zod.infer<typeof FunctionExtensionSchema>
 const FunctionExtensionSchema = BaseSchema.extend({
-  build: zod.object({
-    command: zod
-      .string()
-      .transform((value) => (value.trim() === '' ? undefined : value))
-      .optional(),
-    path: zod.string().optional(),
-    watch: zod.union([zod.string(), zod.string().array()]).optional(),
-    wasm_opt: zod.boolean().optional().default(true),
-  }),
+  build: zod
+    .object({
+      command: zod
+        .string()
+        .transform((value) => (value.trim() === '' ? undefined : value))
+        .optional(),
+      path: zod.string().optional(),
+      watch: zod.union([zod.string(), zod.string().array()]).optional(),
+      wasm_opt: zod.boolean().optional().default(true),
+    })
+    .optional(),
   name: zod.string(),
   type: zod.string(),
   configuration_ui: zod.boolean().optional().default(true),

packages/app/src/cli/services/build/extension.test.ts

@@ -48,7 +48,7 @@ describe('buildFunctionExtension', () => {
 
   test('delegates the build to system when the build command is present', async () => {
     // Given
-    extension.configuration.build.command = './scripts/build.sh argument'
+    extension.configuration.build!.command = './scripts/build.sh argument'
 
     // When
     await expect(
@@ -73,7 +73,7 @@ describe('buildFunctionExtension', () => {
 
   test('fails when is not a JS function and build command is not present', async () => {
     // Given
-    extension.configuration.build.command = undefined
+    extension.configuration.build!.command = undefined
 
     // Then
     await expect(
@@ -91,7 +91,7 @@ describe('buildFunctionExtension', () => {
   test('succeeds when is a JS function and build command is not present', async () => {
     // Given
     extension = await testFunctionExtension({config: defaultConfig, entryPath: 'src/index.js'})
-    extension.configuration.build.command = undefined
+    extension.configuration.build!.command = undefined
 
     // When
     await expect(
@@ -118,7 +118,7 @@ describe('buildFunctionExtension', () => {
   test('succeeds when is a JS function and build command is present', async () => {
     // Given
     extension = await testFunctionExtension({config: defaultConfig, entryPath: 'src/index.js'})
-    extension.configuration.build.command = './scripts/build.sh argument'
+    extension.configuration.build!.command = './scripts/build.sh argument'
 
     // When
     await expect(
@@ -182,7 +182,7 @@ describe('buildFunctionExtension', () => {
   test('skips wasm-opt execution when the disable-wasm-opt is true', async () => {
     // Given
     vi.mocked(fileExistsSync).mockResolvedValue(true)
-    extension.configuration.build.wasm_opt = false
+    extension.configuration.build!.wasm_opt = false
 
     // When
     await expect(
@@ -215,4 +215,83 @@ describe('buildFunctionExtension', () => {
     ).rejects.toThrow(AbortError)
     expect(releaseLock).not.toHaveBeenCalled()
   })
+
+  test('handles function with undefined build config', async () => {
+    // Given
+    const configWithoutBuild = {
+      name: 'MyFunction',
+      type: 'product_discounts',
+      description: '',
+      configuration_ui: true,
+      api_version: '2022-07',
+      metafields: [],
+    } as unknown as FunctionConfigType
+
+    extension = await testFunctionExtension({config: configWithoutBuild, entryPath: 'src/index.js'})
+    vi.mocked(fileExistsSync).mockResolvedValue(true)
+
+    // When
+    await expect(
+      buildFunctionExtension(extension, {
+        stdout,
+        stderr,
+        signal,
+        app,
+        environment: 'production',
+      }),
+    ).resolves.toBeUndefined()
+
+    // Then
+    expect(buildJSFunction).toHaveBeenCalledWith(extension, {
+      stdout,
+      stderr,
+      signal,
+      app,
+      environment: 'production',
+    })
+    expect(releaseLock).toHaveBeenCalled()
+    // wasm_opt should not be called when build config is undefined
+    expect(runWasmOpt).not.toHaveBeenCalled()
+  })
+
+  test('handles function with build config but undefined path', async () => {
+    // Given
+    const configWithoutPath = {
+      name: 'MyFunction',
+      type: 'product_discounts',
+      description: '',
+      build: {
+        command: 'make build',
+        wasm_opt: true,
+        // path is undefined
+      },
+      configuration_ui: true,
+      api_version: '2022-07',
+      metafields: [],
+    } as unknown as FunctionConfigType
+
+    extension = await testFunctionExtension({config: configWithoutPath})
+    vi.mocked(fileExistsSync).mockResolvedValue(true)
+
+    // When
+    await expect(
+      buildFunctionExtension(extension, {
+        stdout,
+        stderr,
+        signal,
+        app,
+        environment: 'production',
+      }),
+    ).resolves.toBeUndefined()
+
+    // Then
+    expect(exec).toHaveBeenCalledWith('make', ['build'], {
+      stdout,
+      stderr,
+      cwd: extension.directory,
+      signal,
+    })
+    expect(releaseLock).toHaveBeenCalled()
+    expect(runWasmOpt).toHaveBeenCalled()
+  })
 })

packages/app/src/cli/services/build/extension.ts

@@ -151,7 +151,7 @@ export async function buildFunctionExtension(
   try {
     const bundlePath = extension.outputPath
     const relativeBuildPath =
-      (extension as ExtensionInstance<FunctionConfigType>).configuration.build.path ?? joinPath('dist', 'index.wasm')
+      (extension as ExtensionInstance<FunctionConfigType>).configuration.build?.path ?? joinPath('dist', 'index.wasm')
 
     extension.outputPath = joinPath(extension.directory, relativeBuildPath)
 
@@ -161,7 +161,7 @@ export async function buildFunctionExtension(
       await buildOtherFunction(extension, options)
     }
 
-    const wasmOpt = (extension as ExtensionInstance<FunctionConfigType>).configuration.build.wasm_opt
+    const wasmOpt = (extension as ExtensionInstance<FunctionConfigType>).configuration.build?.wasm_opt
     if (fileExistsSync(extension.outputPath) && wasmOpt) {
       await runWasmOpt(extension.outputPath)
     }