From 53d1a60d03f37b5ecc42c947597750a437b5d13b Mon Sep 17 00:00:00 2001 From: sswrk Date: Mon, 27 Jul 2026 16:50:36 +0200 Subject: [PATCH] [steps] Expand legacy command/path local functions to a single build step --- .../__tests__/compositeFunctions.test.ts | 3 + .../src/__tests__/compositeFunction.test.ts | 8 +- .../eas-build-job/src/compositeFunction.ts | 3 +- .../steps/src/CompositeFunctionExpander.ts | 107 +++-- packages/steps/src/StepsConfigParser.ts | 12 +- ...rser-composite-functions-expansion-test.ts | 2 +- ...igParser-composite-functions-test-utils.ts | 4 +- .../__tests__/StepsConfigParser-hooks-test.ts | 25 ++ ...StepsConfigParser-legacy-functions-test.ts | 367 ++++++++++++++++++ packages/steps/src/hooks.ts | 12 +- .../__tests__/localCompositeFunctions-test.ts | 71 +++- packages/steps/src/utils/legacyFunction.ts | 76 ++++ .../src/utils/localCompositeFunctions.ts | 39 +- 13 files changed, 673 insertions(+), 56 deletions(-) create mode 100644 packages/steps/src/__tests__/StepsConfigParser-legacy-functions-test.ts create mode 100644 packages/steps/src/utils/legacyFunction.ts diff --git a/packages/build-tools/src/steps/__tests__/compositeFunctions.test.ts b/packages/build-tools/src/steps/__tests__/compositeFunctions.test.ts index 071170536b..bb9d61e1d3 100644 --- a/packages/build-tools/src/steps/__tests__/compositeFunctions.test.ts +++ b/packages/build-tools/src/steps/__tests__/compositeFunctions.test.ts @@ -1,3 +1,5 @@ +import { isLegacyFunctionConfig } from '@expo/eas-build-job'; +import assert from 'assert'; import { promises as fs } from 'fs'; import os from 'os'; import path from 'path'; @@ -43,6 +45,7 @@ describe(buildCompositeFunctionCatalogAsync, () => { expect(Object.keys(catalog)).toEqual(['./.eas/functions/setup']); const action = catalog['./.eas/functions/setup']; + assert(!isLegacyFunctionConfig(action)); expect(action.name).toBe('Setup'); expect(action.runs.steps).toHaveLength(1); expect(action.outputs?.version.value).toBe('${{ steps.read.outputs.version }}'); diff --git a/packages/eas-build-job/src/__tests__/compositeFunction.test.ts b/packages/eas-build-job/src/__tests__/compositeFunction.test.ts index 7b0c55ccbe..d7d52ddebc 100644 --- a/packages/eas-build-job/src/__tests__/compositeFunction.test.ts +++ b/packages/eas-build-job/src/__tests__/compositeFunction.test.ts @@ -127,9 +127,11 @@ describe('LegacyFunctionConfigZ', () => { }); it('rejects a config declaring both command and path', () => { - expect(parseErrorMessages(LegacyFunctionConfigZ, { command: 'echo hi', path: './fn' })).toEqual([ - 'A local function must declare either "command" (a shell script) or "path" (a JavaScript function module), not both.', - ]); + expect(parseErrorMessages(LegacyFunctionConfigZ, { command: 'echo hi', path: './fn' })).toEqual( + [ + 'A local function must declare either "command" (a shell script) or "path" (a JavaScript function module), not both.', + ] + ); }); it('rejects a config declaring neither command nor path', () => { diff --git a/packages/eas-build-job/src/compositeFunction.ts b/packages/eas-build-job/src/compositeFunction.ts index 220bcb93f3..9d7e55c49b 100644 --- a/packages/eas-build-job/src/compositeFunction.ts +++ b/packages/eas-build-job/src/compositeFunction.ts @@ -288,4 +288,5 @@ export function isLegacyFunctionConfig( return config.runs === undefined; } -export type CompositeFunctionCatalog = Record; +/** Local functions of either shape, keyed by their normalized `uses:` path. */ +export type CompositeFunctionCatalog = Record; diff --git a/packages/steps/src/CompositeFunctionExpander.ts b/packages/steps/src/CompositeFunctionExpander.ts index 87f91ca42d..62929d3415 100644 --- a/packages/steps/src/CompositeFunctionExpander.ts +++ b/packages/steps/src/CompositeFunctionExpander.ts @@ -1,23 +1,26 @@ /** - * Expands local composite functions (`uses: ./path/to/function`) into a - * {@link CompositeBuildStep} tree at parse time. + * Expands local functions (`uses: ./path/to/function`) into build steps at parse time. * - * Each call becomes a node with prefixed child ids (`caller__inner`); the parser - * flattens the tree into the workflow. Expanded steps carry a - * {@link BuildStepCompositeFunctionScope} so `${{ steps.* }}` and `${{ inputs.* }}` - * resolve against composite-function-local names. + * A composite function call becomes a {@link CompositeBuildStep} node with prefixed child ids + * (`caller__inner`); the parser flattens the tree into the workflow. Expanded steps carry a + * {@link BuildStepCompositeFunctionScope} so `${{ steps.* }}` and `${{ inputs.* }}` resolve + * against composite-function-local names. A single-step (`command`/`path`) function call becomes + * one ordinary build step keeping the caller's own id. */ import { CompositeFunctionCatalog, CompositeFunctionConfig, FunctionStep, + LegacyFunctionConfig, + LocalFunctionConfig, ShellStep, Step, + isLegacyFunctionConfig, isStepFunctionStep, isStepShellStep, } from '@expo/eas-build-job'; -import { BuildFunctionById } from './BuildFunction'; +import { BuildFunction, BuildFunctionById } from './BuildFunction'; import { BuildFunctionGroupById } from './BuildFunctionGroup'; import { BuildStep, BuildStepOutputAccessor } from './BuildStep'; import { BuildStepCompositeFunctionScope } from './BuildStepCompositeFunctionScope'; @@ -31,6 +34,7 @@ import { import { CompositeBuildStep } from './CompositeBuildStep'; import { BuildConfigError } from './errors'; import { duplicates } from './utils/expodash/duplicates'; +import { createBuildFunctionFromLegacyFunctionConfig } from './utils/legacyFunction'; import { getLocalCompositeFunctionCallWorkingDirectoryError, isLocalCompositeFunctionPath, @@ -45,12 +49,16 @@ export type FunctionMaps = { buildFunctionGroupById: BuildFunctionGroupById; }; -type CompositeFunctionCall = { +export type FunctionMapsWithExpander = FunctionMaps & { + compositeFunctionExpander: CompositeFunctionExpander; +}; + +type LocalFunctionCall = { compositeFunctionPath: string; - /** Caller-assigned id used as prefix for all inner step ids. */ + /** Caller-assigned id; for a composite function also the prefix of all inner step ids. */ syntheticStepId: string; name?: string; - /** Caller-provided input values, consumed when composite function inputs are interpolated. */ + /** Caller-provided input values, consumed when function inputs are interpolated. */ callWith?: Record; callIf?: string; parentScope?: BuildStepCompositeFunctionScope; @@ -64,6 +72,9 @@ type StepOverrides = { }; export class CompositeFunctionExpander { + /** Cached per path: a `BuildFunction` is stateless, and one path may be called many times. */ + private readonly legacyFunctionByPath = new Map(); + constructor( private readonly ctx: BuildStepGlobalContext, private readonly compositeFunctionCatalog: CompositeFunctionCatalog, @@ -78,15 +89,16 @@ export class CompositeFunctionExpander { return this.functionMaps.buildFunctionGroupById; } - public expandCompositeFunctionStep( + /** Steps a local function call expands to: many for a composite function, one otherwise. */ + public expandLocalFunctionStep( step: FunctionStep, - compositeFunctionPath: string, + functionPath: string, syntheticStepId: string - ): CompositeBuildStep { - this.rejectCompositeFunctionCallWorkingDirectory(step); - return this.expand( + ): BuildStep[] { + this.rejectLocalFunctionCallWorkingDirectory(step); + const expanded = this.expandCall( { - compositeFunctionPath, + compositeFunctionPath: functionPath, syntheticStepId, name: step.name, callWith: step.with, @@ -95,21 +107,58 @@ export class CompositeFunctionExpander { }, new Set() ); + return expanded instanceof CompositeBuildStep ? expanded.getFlattenedSteps() : [expanded]; } // The call step expands away; `working_directory` on it would never apply. - private rejectCompositeFunctionCallWorkingDirectory(step: FunctionStep): void { + private rejectLocalFunctionCallWorkingDirectory(step: FunctionStep): void { if (step.working_directory !== undefined) { throw new BuildConfigError(getLocalCompositeFunctionCallWorkingDirectoryError(step.uses)); } } + private expandCall(call: LocalFunctionCall, visited: ReadonlySet): BuildStep { + const localFunction = this.lookupLocalFunction(call.compositeFunctionPath); + if (isLegacyFunctionConfig(localFunction)) { + return this.createLegacyFunctionStep(call, localFunction); + } + return this.expand(call, localFunction, visited); + } + + /** + * A single-step function keeps the caller's id and takes the call-site `if` as its own + * condition; there is no expansion scope to hang it on. + */ + private createLegacyFunctionStep( + call: LocalFunctionCall, + config: LegacyFunctionConfig + ): BuildStep { + const { compositeFunctionPath } = call; + let buildFunction = this.legacyFunctionByPath.get(compositeFunctionPath); + if (!buildFunction) { + buildFunction = createBuildFunctionFromLegacyFunctionConfig(compositeFunctionPath, config); + this.legacyFunctionByPath.set(compositeFunctionPath, buildFunction); + } + return buildFunction.createBuildStepFromFunctionCall(this.ctx, { + id: call.syntheticStepId, + name: call.name, + callInputs: call.callWith, + shell: config.shell, + env: call.inheritedEnv, + ifCondition: call.callIf, + compositeFunctionScope: call.parentScope, + }); + } + // `visited.size` is the current composite function nesting depth. - private expand(call: CompositeFunctionCall, visited: ReadonlySet): CompositeBuildStep { + private expand( + call: LocalFunctionCall, + compositeFunction: CompositeFunctionConfig, + visited: ReadonlySet + ): CompositeBuildStep { const { compositeFunctionPath, syntheticStepId } = call; this.guardAgainstRunawayRecursion(compositeFunctionPath, visited); - const compositeFunction = this.lookupCompositeFunction(compositeFunctionPath); const compositeFunctionDisplayName = call.name ?? compositeFunction.name ?? compositeFunctionPath; const innerSteps = compositeFunction.runs.steps; @@ -184,14 +233,14 @@ export class CompositeFunctionExpander { } } - private lookupCompositeFunction(compositeFunctionPath: string): CompositeFunctionConfig { - const compositeFunction = this.compositeFunctionCatalog[compositeFunctionPath]; - if (!compositeFunction) { + private lookupLocalFunction(compositeFunctionPath: string): LocalFunctionConfig { + const localFunction = this.compositeFunctionCatalog[compositeFunctionPath]; + if (!localFunction) { throw new BuildConfigError( - `Local composite function "${compositeFunctionPath}" does not exist. Expected a "function.yml" (or "function.yaml") file at "${compositeFunctionPath}" relative to the EAS project root (convention: ".eas/functions/").` + `Local function "${compositeFunctionPath}" does not exist. Expected a "function.yml" (or "function.yaml") file at "${compositeFunctionPath}" relative to the EAS project root (convention: ".eas/functions/").` ); } - return compositeFunction; + return localFunction; } private expandInnerStep( @@ -211,8 +260,8 @@ export class CompositeFunctionExpander { if (isStepFunctionStep(innerStep)) { if (isLocalCompositeFunctionPath(innerStep.uses)) { - this.rejectCompositeFunctionCallWorkingDirectory(innerStep); - return this.expandNestedCompositeFunctionCall(innerStep, { + this.rejectLocalFunctionCallWorkingDirectory(innerStep); + return this.expandNestedLocalFunctionCall(innerStep, { compositeFunctionPath: parseLocalCompositeFunctionPath(innerStep.uses), newId, overrides, @@ -243,7 +292,7 @@ export class CompositeFunctionExpander { }; } - private expandNestedCompositeFunctionCall( + private expandNestedLocalFunctionCall( innerStep: FunctionStep, { compositeFunctionPath, @@ -258,8 +307,8 @@ export class CompositeFunctionExpander { scope: BuildStepCompositeFunctionScope; visited: ReadonlySet; } - ): CompositeBuildStep { - return this.expand( + ): BuildStep { + return this.expandCall( { compositeFunctionPath, syntheticStepId: newId, diff --git a/packages/steps/src/StepsConfigParser.ts b/packages/steps/src/StepsConfigParser.ts index ae6bccd2a3..05c09a4aa2 100644 --- a/packages/steps/src/StepsConfigParser.ts +++ b/packages/steps/src/StepsConfigParser.ts @@ -285,13 +285,11 @@ export class StepsConfigParser extends AbstractConfigParser { compositeFunctionExpander: CompositeFunctionExpander ): BuildStep[] { if (isLocalCompositeFunctionPath(step.uses)) { - return compositeFunctionExpander - .expandCompositeFunctionStep( - step, - parseLocalCompositeFunctionPath(step.uses), - BuildStep.getNewId(step.id) - ) - .getFlattenedSteps(); + return compositeFunctionExpander.expandLocalFunctionStep( + step, + parseLocalCompositeFunctionPath(step.uses), + BuildStep.getNewId(step.id) + ); } const buildFunction = compositeFunctionExpander.buildFunctionById[step.uses]; diff --git a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-expansion-test.ts b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-expansion-test.ts index ba1aa7cff4..8373a46f77 100644 --- a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-expansion-test.ts +++ b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-expansion-test.ts @@ -163,7 +163,7 @@ describe('StepsConfigParser local composite functions', () => { parseCompositeFunctions({ steps: [{ uses: './.eas/functions/missing', id: 'x' }], }) - ).rejects.toThrow(/Local composite function ".\/.eas\/functions\/missing"/); + ).rejects.toThrow(/Local function ".\/.eas\/functions\/missing" does not exist/); }); it.each([ diff --git a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-test-utils.ts b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-test-utils.ts index 185139f049..61c29e8a38 100644 --- a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-test-utils.ts +++ b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-test-utils.ts @@ -1,4 +1,4 @@ -import { CompositeFunctionCatalog, CompositeFunctionConfigZ, Step } from '@expo/eas-build-job'; +import { CompositeFunctionCatalog, LocalFunctionConfigZ, Step } from '@expo/eas-build-job'; import { createGlobalContextMock } from './utils/context'; import { BuildFunction } from '../BuildFunction'; @@ -13,7 +13,7 @@ export const SETUP = './.eas/functions/setup'; export function makeCatalog(entries: Record): CompositeFunctionCatalog { const catalog: CompositeFunctionCatalog = {}; for (const [compositeFunctionPath, raw] of Object.entries(entries)) { - catalog[compositeFunctionPath] = CompositeFunctionConfigZ.parse(raw); + catalog[compositeFunctionPath] = LocalFunctionConfigZ.parse(raw); } return catalog; } diff --git a/packages/steps/src/__tests__/StepsConfigParser-hooks-test.ts b/packages/steps/src/__tests__/StepsConfigParser-hooks-test.ts index 29399070a3..b79ecf871d 100644 --- a/packages/steps/src/__tests__/StepsConfigParser-hooks-test.ts +++ b/packages/steps/src/__tests__/StepsConfigParser-hooks-test.ts @@ -633,6 +633,31 @@ describe('StepsConfigParser hooks with composite functions', () => { expect(workflow.buildSteps.map(step => step.displayName)).toEqual(['Install node modules']); }); + it('parses a single-step function hook step into one entry with one step', async () => { + const workflow = await parseWorkflowAsync({ + ctx, + steps: [{ uses: 'eas/install_node_modules' }], + hooks: { + before_install_node_modules: [ + { uses: './.eas/functions/say-hi', id: 'greet', if: '${{ always() }}' }, + ], + }, + compositeFunctionCatalog: makeCatalog({ + './.eas/functions/say-hi': { name: 'Say hi', command: 'echo hi' }, + }), + }); + const anchorHooks = [...workflow.hooksByAnchorStep.values()][0]; + expect(anchorHooks.before).toHaveLength(1); + const [hookStep] = anchorHooks.before[0].steps; + expect(anchorHooks.before[0].steps).toHaveLength(1); + expect(hookStep.id).toBe('greet'); + expect(hookStep.displayName).toBe('Say hi'); + expect(hookStep.command).toBe('echo hi'); + // Unlike a composite call, the authored if: lands on the step itself. + expect(hookStep.ifCondition).toBe('${{ always() }}'); + expect(workflow.buildSteps.map(step => step.displayName)).toEqual(['Install node modules']); + }); + it('does not copy the if condition of a composite hook step onto the entry', async () => { // The authored if: is applied inside the expansion scope, not on the entry. const workflow = await parseWorkflowAsync({ diff --git a/packages/steps/src/__tests__/StepsConfigParser-legacy-functions-test.ts b/packages/steps/src/__tests__/StepsConfigParser-legacy-functions-test.ts new file mode 100644 index 0000000000..653357fd4c --- /dev/null +++ b/packages/steps/src/__tests__/StepsConfigParser-legacy-functions-test.ts @@ -0,0 +1,367 @@ +import fs from 'fs/promises'; +import assert from 'node:assert'; + +import { parseCompositeFunctions } from './StepsConfigParser-composite-functions-test-utils'; +import { getErrorAsync } from './utils/error'; +import { BuildWorkflow } from '../BuildWorkflow'; +import { BuildConfigError, BuildWorkflowError } from '../errors'; + +const SAY_HI = './.eas/functions/say-hi'; + +const createdDirectories: string[] = []; + +// Shell steps spawn in the project target directory, which the mock context does not create. +async function executeWorkflowAsync(workflow: BuildWorkflow): Promise { + const globalCtx = workflow.buildSteps[0].ctx.global; + for (const directory of [ + globalCtx.defaultWorkingDirectory, + globalCtx.stepsInternalBuildDirectory, + ]) { + await fs.mkdir(directory, { recursive: true }); + createdDirectories.push(directory); + } + await workflow.executeAsync(); +} + +afterEach(async () => { + await Promise.all( + createdDirectories.map(directory => fs.rm(directory, { recursive: true, force: true })) + ); + createdDirectories.length = 0; +}); + +describe('StepsConfigParser local single-step functions', () => { + describe('expansion', () => { + it('expands a command function into one step keeping the caller id', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { name: 'Say hi', command: 'echo hi' } }, + steps: [ + { + uses: SAY_HI, + id: 'greet', + name: 'Greet the world', + env: { GREETING: 'Hi' }, + if: '${{ always() }}', + }, + ], + }); + + expect(workflow.buildSteps).toHaveLength(1); + const [step] = workflow.buildSteps; + expect(step.id).toBe('greet'); + expect(step.displayName).toBe('Greet the world'); + expect(step.command).toBe('echo hi'); + expect(step.fn).toBeUndefined(); + expect(step.ifCondition).toBe('${{ always() }}'); + expect(step.stepEnvOverrides).toEqual({ GREETING: 'Hi' }); + }); + + it('falls back to the function name and then to the step id for the display name', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { name: 'Say hi', command: 'echo hi' }, + './.eas/functions/anonymous': { command: 'echo anonymous' }, + }, + steps: [ + { uses: SAY_HI, id: 'greet' }, + { uses: './.eas/functions/anonymous', id: 'anonymous' }, + ], + }); + + expect(workflow.buildSteps.map(step => step.displayName)).toEqual(['Say hi', 'anonymous']); + }); + + it('generates a step id when the caller has none', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi' } }, + steps: [{ uses: SAY_HI }], + }); + + expect(workflow.buildSteps[0].id).toMatch(/^step-\d{3,}$/); + }); + + it('expands repeated calls to the same function into separate steps', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi' } }, + steps: [ + { uses: SAY_HI, id: 'first' }, + { uses: SAY_HI, id: 'second' }, + ], + }); + + expect(workflow.buildSteps.map(step => step.id)).toEqual(['first', 'second']); + }); + + it('forwards the function-level shell to the step', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi', shell: 'sh' } }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }); + + expect(workflow.buildSteps[0].shell).toBe('sh'); + }); + + it('expands a path function into a step calling the module', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { path: '/tmp/functions/say-hi' } }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }); + + const [step] = workflow.buildSteps; + expect(step.command).toBeUndefined(); + expect(step.fn).toBeDefined(); + }); + + it('rejects working_directory on a step that calls a single-step function', async () => { + await expect( + parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi' } }, + steps: [{ uses: SAY_HI, id: 'greet', working_directory: 'packages/app' }], + }) + ).rejects.toThrow( + /"working_directory" is not supported on a step that calls a local function/ + ); + }); + + it('throws a clear error for a function missing from the catalog', async () => { + await expect( + parseCompositeFunctions({ steps: [{ uses: SAY_HI, id: 'greet' }] }) + ).rejects.toThrow(/Local function ".\/.eas\/functions\/say-hi" does not exist/); + }); + }); + + describe('inputs', () => { + it('passes caller values to the function inputs', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { + inputs: ['name'], + outputs: ['greeting'], + command: 'set-output greeting "Hi, ${ inputs.name }!"', + }, + }, + steps: [{ uses: SAY_HI, id: 'greet', with: { name: 'World' } }], + }); + await executeWorkflowAsync(workflow); + + expect(workflow.buildSteps[0].getOutputValueByName('greeting')).toBe('Hi, World!'); + }); + + it('applies the declared default when the caller omits a value', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { + inputs: [{ name: 'name', type: 'string', default_value: 'World', required: false }], + outputs: ['greeting'], + command: 'set-output greeting "Hi, ${ inputs.name }!"', + }, + }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }); + await executeWorkflowAsync(workflow); + + expect(workflow.buildSteps[0].getOutputValueByName('greeting')).toBe('Hi, World!'); + }); + + it('treats shorthand inputs as required, unlike composite function inputs', async () => { + const error = await getErrorAsync(() => + parseCompositeFunctions({ + catalog: { [SAY_HI]: { inputs: ['name'], command: 'echo hi' } }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }) + ); + + expect(error).toBeInstanceOf(BuildWorkflowError); + assert(error instanceof BuildWorkflowError); + expect(error.errors[0].message).toBe( + 'Input parameter "name" for step "greet" is required but it was not set.' + ); + }); + + it('accepts an input declared as not required', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { + inputs: [{ name: 'name', type: 'string', required: false }], + command: 'echo hi', + }, + }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }); + + expect(workflow.buildSteps).toHaveLength(1); + }); + + it('rejects a value outside the declared allowed values', async () => { + const error = await getErrorAsync(() => + parseCompositeFunctions({ + catalog: { + [SAY_HI]: { + inputs: [ + { + name: 'platform', + type: 'string', + default_value: 'ios', + allowed_values: ['ios', 'android'], + }, + ], + command: 'echo hi', + }, + }, + steps: [{ uses: SAY_HI, id: 'greet', with: { platform: 'web' } }], + }) + ); + + expect(error).toBeInstanceOf(BuildWorkflowError); + assert(error instanceof BuildWorkflowError); + expect(error.errors[0].message).toBe( + 'Input parameter "platform" for step "greet" is set to "web" which is not one of the allowed values: "ios", "android".' + ); + }); + }); + + describe('outputs', () => { + it('exposes declared outputs to later steps', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { outputs: ['version'], command: 'set-output version "1.0.0"' }, + }, + steps: [ + { uses: SAY_HI, id: 'read' }, + { + id: 'copy', + run: 'set-output copied "${{ steps.read.outputs.version }}"', + outputs: [{ name: 'copied', required: true }], + }, + ], + }); + await executeWorkflowAsync(workflow); + + expect(workflow.buildSteps[0].getOutputValueByName('version')).toBe('1.0.0'); + expect(workflow.buildSteps[1].getOutputValueByName('copied')).toBe('1.0.0'); + }); + + it('treats shorthand outputs as required', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { outputs: ['version'], command: 'echo hi' } }, + steps: [{ uses: SAY_HI, id: 'read' }], + }); + + await expect(executeWorkflowAsync(workflow)).rejects.toThrow( + /Some required outputs have not been set: "version"/ + ); + }); + + it('accepts an output declared as not required', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SAY_HI]: { outputs: [{ name: 'version', required: false }], command: 'echo hi' }, + }, + steps: [{ uses: SAY_HI, id: 'read' }], + }); + + await expect(executeWorkflowAsync(workflow)).resolves.toBeUndefined(); + }); + }); + + describe('supported platforms', () => { + it('rejects a function that does not support the runtime platform', async () => { + const error = await getErrorAsync(() => + parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi', supported_platforms: ['darwin'] } }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }) + ); + + expect(error).toBeInstanceOf(BuildWorkflowError); + assert(error instanceof BuildWorkflowError); + expect(error.errors[0].message).toBe( + 'Step "greet" is not allowed on platform "linux". Allowed platforms for this step are: "darwin".' + ); + }); + + it('accepts a function that supports the runtime platform', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { [SAY_HI]: { command: 'echo hi', supported_platforms: ['linux'] } }, + steps: [{ uses: SAY_HI, id: 'greet' }], + }); + + expect(workflow.buildSteps).toHaveLength(1); + }); + }); + + describe('inside a composite function', () => { + it('expands a single-step function called from a composite function', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + inputs: [{ name: 'name', type: 'string', default_value: 'World' }], + outputs: { greeting: { value: '${{ steps.inner.outputs.greeting }}' } }, + runs: { + steps: [{ id: 'inner', uses: SAY_HI, with: { name: '${{ inputs.name }}' } }], + }, + }, + [SAY_HI]: { + inputs: ['name'], + outputs: ['greeting'], + command: 'set-output greeting "Hi, ${ inputs.name }!"', + }, + }, + steps: [{ uses: './.eas/functions/outer', id: 'outer', with: { name: 'Expo' } }], + }); + await executeWorkflowAsync(workflow); + + const [innerStep, outputsStep] = workflow.buildSteps; + expect(innerStep.id).toBe('outer__inner'); + expect(innerStep.getOutputValueByName('greeting')).toBe('Hi, Expo!'); + expect(outputsStep.id).toBe('outer'); + expect(outputsStep.getOutputValueByName('greeting')).toBe('Hi, Expo!'); + }); + + it('exposes the outputs of a nested single-step function to its siblings', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + runs: { + steps: [ + { id: 'inner', uses: SAY_HI }, + { + id: 'copy', + run: 'set-output copied "${{ steps.inner.outputs.version }}"', + outputs: [{ name: 'copied', required: true }], + }, + ], + }, + }, + [SAY_HI]: { outputs: ['version'], command: 'set-output version "1.0.0"' }, + }, + steps: [{ uses: './.eas/functions/outer', id: 'outer' }], + }); + await executeWorkflowAsync(workflow); + + expect(workflow.buildSteps.map(step => step.id)).toEqual(['outer__inner', 'outer__copy']); + expect(workflow.buildSteps[1].getOutputValueByName('copied')).toBe('1.0.0'); + }); + + it('rejects working_directory on a nested call to a single-step function', async () => { + const error = await getErrorAsync(() => + parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + runs: { + steps: [{ id: 'inner', uses: SAY_HI, working_directory: 'packages/app' }], + }, + }, + [SAY_HI]: { command: 'echo hi' }, + }, + steps: [{ uses: './.eas/functions/outer', id: 'outer' }], + }) + ); + + expect(error).toBeInstanceOf(BuildConfigError); + expect(error.message).toMatch( + /"working_directory" is not supported on a step that calls a local function/ + ); + }); + }); +}); diff --git a/packages/steps/src/hooks.ts b/packages/steps/src/hooks.ts index f527b885fd..1595274ac4 100644 --- a/packages/steps/src/hooks.ts +++ b/packages/steps/src/hooks.ts @@ -138,13 +138,11 @@ export function constructHookEntriesFromValidatedSteps( } if (isLocalCompositeFunctionPath(step.uses)) { entries.push({ - steps: compositeFunctionExpander - .expandCompositeFunctionStep( - step, - parseLocalCompositeFunctionPath(step.uses), - BuildStep.getNewId(step.id) - ) - .getFlattenedSteps(), + steps: compositeFunctionExpander.expandLocalFunctionStep( + step, + parseLocalCompositeFunctionPath(step.uses), + BuildStep.getNewId(step.id) + ), }); continue; } diff --git a/packages/steps/src/utils/__tests__/localCompositeFunctions-test.ts b/packages/steps/src/utils/__tests__/localCompositeFunctions-test.ts index ea552155ad..b5f0406ce3 100644 --- a/packages/steps/src/utils/__tests__/localCompositeFunctions-test.ts +++ b/packages/steps/src/utils/__tests__/localCompositeFunctions-test.ts @@ -1,10 +1,11 @@ -import { CompositeFunctionConfigZ } from '@expo/eas-build-job'; +import { CompositeFunctionConfigZ, LocalFunctionConfigZ } from '@expo/eas-build-job'; import path from 'path'; import { buildCompositeFunctionCatalogFromStepsAsync, isLocalCompositeFunctionPath, parseLocalCompositeFunctionPath, + resolveLegacyFunctionModulePath, resolveLocalCompositeFunctionPath, } from '../localCompositeFunctions'; @@ -154,6 +155,39 @@ describe(buildCompositeFunctionCatalogFromStepsAsync, () => { expect(Object.keys(catalog).sort()).toEqual(paths.sort()); }); + it('loads a single-step function without recursing into it', async () => { + const loadedPaths: string[] = []; + const catalog = await buildCompositeFunctionCatalogFromStepsAsync({ + rootSteps: [{ uses: './.eas/functions/say-hi' }], + loadCompositeFunction: async compositeFunctionPath => { + loadedPaths.push(compositeFunctionPath); + return LocalFunctionConfigZ.parse({ command: 'echo hi' }); + }, + }); + + expect(loadedPaths).toEqual(['./.eas/functions/say-hi']); + expect(Object.keys(catalog)).toEqual(['./.eas/functions/say-hi']); + }); + + it('loads a single-step function referenced from inside a composite function', async () => { + const catalog = await buildCompositeFunctionCatalogFromStepsAsync({ + rootSteps: [{ uses: './.eas/functions/outer' }], + loadCompositeFunction: async compositeFunctionPath => { + if (compositeFunctionPath === './.eas/functions/outer') { + return LocalFunctionConfigZ.parse({ + runs: { steps: [{ uses: './.eas/functions/say-hi' }] }, + }); + } + return LocalFunctionConfigZ.parse({ command: 'echo hi' }); + }, + }); + + expect(Object.keys(catalog).sort()).toEqual([ + './.eas/functions/outer', + './.eas/functions/say-hi', + ]); + }); + it('collects normalized action paths from steps', async () => { const loadedPaths: string[] = []; await buildCompositeFunctionCatalogFromStepsAsync({ @@ -235,3 +269,38 @@ describe(resolveLocalCompositeFunctionPath, () => { ); }); }); + +describe(resolveLegacyFunctionModulePath, () => { + const projectRoot = path.resolve('/tmp/project'); + + it('resolves a relative module path against the function directory', () => { + expect( + resolveLegacyFunctionModulePath({ + projectRoot, + functionPath: './.eas/functions/say-hi', + modulePath: './my-function', + }) + ).toBe(path.join(projectRoot, '.eas', 'functions', 'say-hi', 'my-function')); + }); + + it('resolves a module path pointing outside the function directory', () => { + expect( + resolveLegacyFunctionModulePath({ + projectRoot, + functionPath: './.eas/functions/say-hi', + modulePath: '../shared/my-function', + }) + ).toBe(path.join(projectRoot, '.eas', 'functions', 'shared', 'my-function')); + }); + + it('passes an absolute module path through', () => { + const absolutePath = path.resolve('/opt/functions/say-hi'); + expect( + resolveLegacyFunctionModulePath({ + projectRoot, + functionPath: './.eas/functions/say-hi', + modulePath: absolutePath, + }) + ).toBe(absolutePath); + }); +}); diff --git a/packages/steps/src/utils/legacyFunction.ts b/packages/steps/src/utils/legacyFunction.ts new file mode 100644 index 0000000000..1c46e9f091 --- /dev/null +++ b/packages/steps/src/utils/legacyFunction.ts @@ -0,0 +1,76 @@ +/** + * Maps a single-step local function config (`command` or `path` in `function.yml`, the shape + * custom build functions use in `.eas/build/*.yml`) onto a {@link BuildFunction}, so calling one + * from a workflow runs it exactly like a custom build does. + * + * Legacy semantics are preserved: inputs and outputs are required unless declared otherwise, + * which is the opposite of the composite function default. + */ +import { LegacyFunctionConfig } from '@expo/eas-build-job'; + +import { BuildFunction } from '../BuildFunction'; +import { BuildRuntimePlatform } from '../BuildRuntimePlatform'; +import { + BuildStepInput, + BuildStepInputProvider, + BuildStepInputValueTypeName, +} from '../BuildStepInput'; +import { BuildStepOutput, BuildStepOutputProvider } from '../BuildStepOutput'; + +type LegacyFunctionInput = NonNullable[number]; +type LegacyFunctionOutput = NonNullable[number]; +type LegacyFunctionPlatform = NonNullable[number]; + +const INPUT_VALUE_TYPE_NAMES = { + string: BuildStepInputValueTypeName.STRING, + boolean: BuildStepInputValueTypeName.BOOLEAN, + number: BuildStepInputValueTypeName.NUMBER, + json: BuildStepInputValueTypeName.JSON, +} satisfies Record['type'], BuildStepInputValueTypeName>; + +const RUNTIME_PLATFORMS = { + darwin: BuildRuntimePlatform.DARWIN, + linux: BuildRuntimePlatform.LINUX, +} satisfies Record; + +/** `config.path`, when set, must already be absolute (the loaders resolve it). */ +export function createBuildFunctionFromLegacyFunctionConfig( + functionPath: string, + config: LegacyFunctionConfig +): BuildFunction { + return new BuildFunction({ + id: functionPath, + name: config.name, + command: config.command, + customFunctionModulePath: config.path, + shell: config.shell, + supportedRuntimePlatforms: config.supported_platforms?.map( + platform => RUNTIME_PLATFORMS[platform] + ), + inputProviders: config.inputs?.map(createInputProvider), + outputProviders: config.outputs?.map(createOutputProvider), + }); +} + +function createInputProvider(input: LegacyFunctionInput): BuildStepInputProvider { + if (typeof input === 'string') { + return BuildStepInput.createProvider({ + id: input, + required: true, + allowedValueTypeName: BuildStepInputValueTypeName.STRING, + }); + } + return BuildStepInput.createProvider({ + id: input.name, + required: input.required ?? true, + defaultValue: input.default_value, + allowedValues: input.allowed_values, + allowedValueTypeName: INPUT_VALUE_TYPE_NAMES[input.type], + }); +} + +function createOutputProvider(output: LegacyFunctionOutput): BuildStepOutputProvider { + return typeof output === 'string' + ? BuildStepOutput.createProvider({ id: output, required: true }) + : BuildStepOutput.createProvider({ id: output.name, required: output.required ?? true }); +} diff --git a/packages/steps/src/utils/localCompositeFunctions.ts b/packages/steps/src/utils/localCompositeFunctions.ts index aff386cea4..c90ef2732e 100644 --- a/packages/steps/src/utils/localCompositeFunctions.ts +++ b/packages/steps/src/utils/localCompositeFunctions.ts @@ -1,9 +1,14 @@ -import { CompositeFunctionCatalog, CompositeFunctionConfig, Step } from '@expo/eas-build-job'; +import { + CompositeFunctionCatalog, + LocalFunctionConfig, + Step, + isLegacyFunctionConfig, +} from '@expo/eas-build-job'; import path from 'path'; import { BuildConfigError } from '../errors'; -// Local composite functions referenced via `uses: ./path` or `uses: ../path` in EAS workflows. +// Local functions referenced via `uses: ./path` or `uses: ../path` in EAS workflows. // Not supported in `.eas/build/*.yml` custom build configs. const JOB_CONTEXT_INTERPOLATION_REGEXP = /\$\{\{(.+?)\}\}/; @@ -39,16 +44,16 @@ export function isLocalCompositeFunctionPath(uses: string): boolean { } export function getLocalCompositeFunctionCallWorkingDirectoryError(uses: string): string { - return `"working_directory" is not supported on a step that calls a local composite function ("uses: ${uses.trim()}"). Set "working_directory" on the steps inside the composite function instead.`; + return `"working_directory" is not supported on a step that calls a local function ("uses: ${uses.trim()}"). For a composite function, set "working_directory" on the steps inside it instead.`; } -/** Loads only composite functions transitively referenced by `rootSteps`. Unreferenced files are ignored. */ +/** Loads only functions transitively referenced by `rootSteps`. Unreferenced files are ignored. */ export async function buildCompositeFunctionCatalogFromStepsAsync({ rootSteps, loadCompositeFunction, }: { rootSteps: readonly Step[]; - loadCompositeFunction: (compositeFunctionPath: string) => Promise; + loadCompositeFunction: (compositeFunctionPath: string) => Promise; }): Promise { const catalog: CompositeFunctionCatalog = {}; @@ -60,6 +65,10 @@ export async function buildCompositeFunctionCatalogFromStepsAsync({ const config = await loadCompositeFunction(compositeFunctionPath); catalog[compositeFunctionPath] = config; + // Single-step functions are leaves: they have no steps that could reference other functions. + if (isLegacyFunctionConfig(config)) { + return; + } for (const nestedPath of collectLocalCompositeFunctionPathsFromSteps(config.runs.steps)) { await loadRecursiveAsync(nestedPath); } @@ -79,6 +88,26 @@ export function resolveLocalCompositeFunctionPath( return path.resolve(projectRoot, compositeFunctionPath); } +/** + * Resolves the `path` of a single-step local function against the function's own directory, the + * way a `.eas/build` config resolves it against the config file. Shared by every loader so the + * two cannot drift. + */ +export function resolveLegacyFunctionModulePath({ + projectRoot, + functionPath, + modulePath, +}: { + projectRoot: string; + functionPath: string; + modulePath: string; +}): string { + if (path.isAbsolute(modulePath)) { + return modulePath; + } + return path.resolve(resolveLocalCompositeFunctionPath(projectRoot, functionPath), modulePath); +} + function collectLocalCompositeFunctionPathsFromSteps(steps: readonly Step[]): Set { const paths = new Set(); for (const step of steps) {