Apply review suggestions

packages/build-tools/src/context.ts

@@ -2,24 +2,23 @@ import path from 'path';
 
 import { ExpoConfig } from '@expo/config';
 import {
-  ManagedArtifactType,
   BuildPhase,
   BuildPhaseResult,
   BuildPhaseStats,
   Env,
   EnvironmentSecretType,
+  GenericArtifactType,
   Job,
   LogMarker,
+  ManagedArtifactType,
   Metadata,
   errors,
-  GenericArtifactType,
   isGenericArtifact,
 } from '@expo/eas-build-job';
 import { BuildTrigger } from '@expo/eas-build-job/dist/common';
 import { bunyan } from '@expo/logger';
-import { SpawnOptions, SpawnPromise, SpawnResult } from '@expo/turtle-spawn';
-import fs from 'fs-extra';
 import { DynamicCacheManager } from '@expo/steps';
+import fs from 'fs-extra';
 
 import { resolveBuildPhaseErrorAsync } from './buildErrors/detectError';
 import { readAppConfig } from './utils/appConfig';
@@ -78,11 +77,6 @@ export class BuildContext<TJob extends Job = Job> {
   /**
    * @deprecated
    */
-  public readonly runGlobalExpoCliCommand: (
-    args: string[],
-    options: SpawnOptions,
-    npmVersionAtLeast7: boolean
-  ) => SpawnPromise<SpawnResult>;
   public readonly reportError?: (
     msg: string,
     err?: Error,

packages/build-tools/src/customBuildContext.ts

@@ -13,9 +13,9 @@ import {
 } from '@expo/eas-build-job';
 import { bunyan } from '@expo/logger';
 import {
-  ExternalBuildContextProvider,
   BuildRuntimePlatform,
   DynamicCacheManager,
+  ExternalBuildContextProvider,
 } from '@expo/steps';
 
 import { ArtifactToUpload, BuildContext, CacheManager } from './context';

packages/build-tools/src/steps/easFunctions.ts

@@ -2,30 +2,30 @@ import { BuildFunction } from '@expo/steps';
 
 import { CustomBuildContext } from '../customBuildContext';
 
-import { createUploadArtifactBuildFunction } from './functions/uploadArtifact';
+import { createRestoreCacheBuildFunction, createSaveCacheBuildFunction } from './functions/cache';
+import { calculateEASUpdateRuntimeVersionFunction } from './functions/calculateEASUpdateRuntimeVersion';
 import { createCheckoutBuildFunction } from './functions/checkout';
-import { createSetUpNpmrcBuildFunction } from './functions/useNpmToken';
-import { createInstallNodeModulesBuildFunction } from './functions/installNodeModules';
-import { createPrebuildBuildFunction } from './functions/prebuild';
-import { createFindAndUploadBuildArtifactsBuildFunction } from './functions/findAndUploadBuildArtifacts';
-import { createSaveCacheBuildFunction, createRestoreCacheBuildFunction } from './functions/cache';
-import { configureEASUpdateIfInstalledFunction } from './functions/configureEASUpdateIfInstalled';
-import { injectAndroidCredentialsFunction } from './functions/injectAndroidCredentials';
 import { configureAndroidVersionFunction } from './functions/configureAndroidVersion';
-import { runGradleFunction } from './functions/runGradle';
-import { resolveAppleTeamIdFromCredentialsFunction } from './functions/resolveAppleTeamIdFromCredentials';
+import { configureEASUpdateIfInstalledFunction } from './functions/configureEASUpdateIfInstalled';
 import { configureIosCredentialsFunction } from './functions/configureIosCredentials';
 import { configureIosVersionFunction } from './functions/configureIosVersion';
+import { createFindAndUploadBuildArtifactsBuildFunction } from './functions/findAndUploadBuildArtifacts';
 import { generateGymfileFromTemplateFunction } from './functions/generateGymfileFromTemplate';
-import { runFastlaneFunction } from './functions/runFastlane';
-import { createStartAndroidEmulatorBuildFunction } from './functions/startAndroidEmulator';
-import { createStartIosSimulatorBuildFunction } from './functions/startIosSimulator';
-import { createInstallMaestroBuildFunction } from './functions/installMaestro';
 import { createGetCredentialsForBuildTriggeredByGithubIntegration } from './functions/getCredentialsForBuildTriggeredByGitHubIntegration';
+import { injectAndroidCredentialsFunction } from './functions/injectAndroidCredentials';
+import { createInstallMaestroBuildFunction } from './functions/installMaestro';
+import { createInstallNodeModulesBuildFunction } from './functions/installNodeModules';
 import { createInstallPodsBuildFunction } from './functions/installPods';
-import { createSendSlackMessageFunction } from './functions/sendSlackMessage';
+import { createPrebuildBuildFunction } from './functions/prebuild';
+import { resolveAppleTeamIdFromCredentialsFunction } from './functions/resolveAppleTeamIdFromCredentials';
 import { createResolveBuildConfigBuildFunction } from './functions/resolveBuildConfig';
-import { calculateEASUpdateRuntimeVersionFunction } from './functions/calculateEASUpdateRuntimeVersion';
+import { runFastlaneFunction } from './functions/runFastlane';
+import { runGradleFunction } from './functions/runGradle';
+import { createSendSlackMessageFunction } from './functions/sendSlackMessage';
+import { createStartAndroidEmulatorBuildFunction } from './functions/startAndroidEmulator';
+import { createStartIosSimulatorBuildFunction } from './functions/startIosSimulator';
+import { createUploadArtifactBuildFunction } from './functions/uploadArtifact';
+import { createSetUpNpmrcBuildFunction } from './functions/useNpmToken';
 
 export function getEasFunctions(ctx: CustomBuildContext): BuildFunction[] {
   const functions = [
@@ -34,8 +34,7 @@ export function getEasFunctions(ctx: CustomBuildContext): BuildFunction[] {
     createSetUpNpmrcBuildFunction(),
     createInstallNodeModulesBuildFunction(),
     createPrebuildBuildFunction(),
-    createSaveCacheBuildFunction(ctx),
-    createRestoreCacheBuildFunction(ctx),
+
     configureEASUpdateIfInstalledFunction(),
     injectAndroidCredentialsFunction(),
     configureAndroidVersionFunction(),
@@ -58,6 +57,8 @@ export function getEasFunctions(ctx: CustomBuildContext): BuildFunction[] {
   if (ctx.hasBuildJob()) {
     functions.push(
       ...[
+        createSaveCacheBuildFunction(ctx),
+        createRestoreCacheBuildFunction(ctx),
         createFindAndUploadBuildArtifactsBuildFunction(ctx),
         createResolveBuildConfigBuildFunction(ctx),
         createGetCredentialsForBuildTriggeredByGithubIntegration(ctx),

packages/build-tools/src/steps/functions/__tests__/cache.test.ts

@@ -29,7 +29,6 @@ const buildCtx = new BuildContext(createTestIosJob({}), {
   uploadArtifact: jest.fn(),
   workingdir: '',
   dynamicCacheManager,
-  runGlobalExpoCliCommand: jest.fn(),
 });
 const customContext = new CustomBuildContext(buildCtx);
 

packages/build-tools/src/steps/functions/cache.ts

@@ -8,7 +8,7 @@ import {
 
 import { CustomBuildContext } from '../../customBuildContext';
 
-export function createRestoreCacheBuildFunction(ctx: CustomBuildContext): BuildFunction {
+export function createRestoreCacheBuildFunction(ctx: CustomBuildContext<BuildJob>): BuildFunction {
   return new BuildFunction({
     namespace: 'eas',
     id: 'restore-cache',
@@ -51,9 +51,10 @@ export function createRestoreCacheBuildFunction(ctx: CustomBuildContext): BuildF
         return;
       }
 
-      const job = stepsCtx.global.staticContext.job as BuildJob;
-      const cache = job.cache;
+      const { cache } = ctx.job;
       if (!cache) {
+        stepsCtx.logger.warn('Cache is not available, skipping...');
+
         return;
       }
 
@@ -73,7 +74,7 @@ export function createRestoreCacheBuildFunction(ctx: CustomBuildContext): BuildF
   });
 }
 
-export function createSaveCacheBuildFunction(ctx: CustomBuildContext): BuildFunction {
+export function createSaveCacheBuildFunction(ctx: CustomBuildContext<BuildJob>): BuildFunction {
   return new BuildFunction({
     namespace: 'eas',
     id: 'save-cache',

packages/steps/build.sh

@@ -11,7 +11,6 @@ if [[ "$npm_lifecycle_event" == "prepack" ]]; then
   echo 'Removing "dist_commonjs" and "dist_esm" folders...'
   rm -rf dist_commonjs dist_esm
 fi
-rm -rf dist_commonjs dist_esm
 
 echo 'Compiling TypeScript to JavaScript...'
 node_modules/.bin/tsc --project tsconfig.build.json

packages/steps/package.json

@@ -32,7 +32,6 @@
   "license": "BUSL-1.1",
   "devDependencies": {
     "@jest/globals": "^29.7.0",
-    "@types/glob": "^8.1.0",
     "@types/jest": "^29.5.11",
     "@types/lodash.clonedeep": "^4.5.9",
     "@types/lodash.get": "^4.4.9",

packages/steps/src/BuildStepInput.ts

@@ -8,9 +8,9 @@ import callInputFunctionAsync from './inputFunctions.js';
 import {
   BUILD_STEP_FUNCTION_EXPRESSION_REGEXP,
   BUILD_STEP_OR_BUILD_GLOBAL_CONTEXT_REFERENCE_REGEX,
-  iterateWithFunctions,
   interpolateWithFunctionsAsync,
   interpolateWithOutputs,
+  iterateWithFunctions,
 } from './utils/template.js';
 
 export enum BuildStepInputValueTypeName {
@@ -57,6 +57,18 @@ interface BuildStepInputParams<T extends BuildStepInputValueTypeName, R extends
   stepDisplayName: string;
 }
 
+interface ValidateFunctionError {
+  error: string;
+  templateFunction?: never;
+}
+
+interface ValidateFunctionSuccess {
+  error?: never;
+  templateFunction: string;
+}
+
+type ValidateFunctionInput = ValidateFunctionError | ValidateFunctionSuccess;
+
 export interface SerializedBuildStepInput<
   T extends BuildStepInputValueTypeName = BuildStepInputValueTypeName,
   R extends boolean = boolean,
@@ -109,7 +121,10 @@ export class BuildStepInput<
   }
 
   public isFunctionCall(): boolean {
-    return this.rawValue?.toString().match(BUILD_STEP_FUNCTION_EXPRESSION_REGEXP) !== null;
+    if (!this.rawValue) {
+      return false;
+    }
+    return this.rawValue.toString().match(BUILD_STEP_FUNCTION_EXPRESSION_REGEXP) !== null;
   }
 
   private requiresInterpolation(rawValue: any): rawValue is string {
@@ -120,22 +135,20 @@ export class BuildStepInput<
     );
   }
 
-  public validateFunctions(
-    fn: (error: string | null, f: string | null, args: string[]) => any
-  ): BuildConfigError[] {
+  public validateFunctions(fn: (input: ValidateFunctionInput) => any): BuildConfigError[] {
     const rawValue = this._value ?? this.defaultValue;
     const errors = [];
     if (this.requiresInterpolation(rawValue) && this.isFunctionCall()) {
       try {
-        iterateWithFunctions(rawValue, (fun, args) => {
-          const error = fn(null, fun, args);
+        iterateWithFunctions(rawValue, (templateFunction) => {
+          const error = fn({ templateFunction });
           if (error) {
             errors.push(error);
           }
         });
       } catch (e) {
         if (e instanceof BuildConfigError) {
-          errors.push(fn(e.message, null, []));
+          errors.push(fn({ error: e.message }));
         } else {
           throw e;
         }
@@ -147,12 +160,9 @@ export class BuildStepInput<
   public async prepareValueAsync(): Promise<void> {
     const rawValue = this._value ?? this.defaultValue;
     if (this.requiresInterpolation(rawValue) && this.isFunctionCall()) {
-      this._computedValue = (await interpolateWithFunctionsAsync(
-        rawValue,
-        (fn: string, args: string[]) => {
-          return callInputFunctionAsync(fn, args, this.ctx);
-        }
-      )) as BuildStepInputValueType<T>;
+      this._computedValue = (await interpolateWithFunctionsAsync(rawValue, (templateFunction) => {
+        return callInputFunctionAsync(templateFunction, this.ctx);
+      })) as BuildStepInputValueType<T>;
     }
   }
 

packages/steps/src/BuildWorkflowValidator.ts

@@ -6,7 +6,7 @@ import { BuildStep } from './BuildStep.js';
 import { BuildStepInputValueTypeName } from './BuildStepInput.js';
 import { BuildWorkflow } from './BuildWorkflow.js';
 import { BuildConfigError, BuildWorkflowError } from './errors.js';
-import * as inputFunctions from './inputFunctions.js';
+import { validateInputFunction } from './inputFunctions.js';
 import { duplicates } from './utils/expodash/duplicates.js';
 import { nullthrows } from './utils/nullthrows.js';
 import { findOutputPaths } from './utils/template.js';
@@ -77,22 +77,29 @@ export class BuildWorkflowValidator {
 
         if (currentStepInput.isFunctionCall()) {
           errors.push(
-            ...currentStepInput.validateFunctions((error, fn, args) => {
+            ...currentStepInput.validateFunctions(({ error, templateFunction }) => {
               if (error) {
                 return new BuildConfigError(
                   `Input parameter "${currentStepInput.id}" for step "${currentStep.displayName}" ${error}.`
                 );
               }
-              if (fn && fn in inputFunctions) {
+              if (!templateFunction) {
                 return null;
               }
-              return new BuildConfigError(
-                `Input parameter "${currentStepInput.id}" for step "${
-                  currentStep.displayName
-                }" is set to "\${ ${fn}(${args
-                  .map((i) => `"${i}"`)
-                  .join(',')}) }" which is not a valid build-in function name.`
-              );
+              try {
+                validateInputFunction(templateFunction);
+              } catch (err: any) {
+                if (err.message.startsWith('Invalid identifier')) {
+                  return new BuildConfigError(
+                    `Input parameter "${currentStepInput.id}" for step "${currentStep.displayName}" is set to "\${ ${templateFunction} }" which is not a valid built-in function name.`
+                  );
+                } else {
+                  return new BuildConfigError(
+                    `Input parameter "${currentStepInput.id}" for step "${currentStep.displayName}" contains an error in expression '${templateFunction}': ${err.message}.`
+                  );
+                }
+              }
+              return null;
             })
           );
         }

packages/steps/src/__tests__/BuildWorkflowValidator-test.ts

@@ -1,13 +1,13 @@
 import assert from 'assert';
 
+import { BuildFunction } from '../BuildFunction.js';
+import { BuildRuntimePlatform } from '../BuildRuntimePlatform.js';
 import { BuildStep, BuildStepFunction } from '../BuildStep.js';
 import { BuildStepInput, BuildStepInputValueTypeName } from '../BuildStepInput.js';
 import { BuildStepOutput } from '../BuildStepOutput.js';
 import { BuildWorkflow } from '../BuildWorkflow.js';
 import { BuildWorkflowValidator } from '../BuildWorkflowValidator.js';
 import { BuildConfigError, BuildWorkflowError } from '../errors.js';
-import { BuildRuntimePlatform } from '../BuildRuntimePlatform.js';
-import { BuildFunction } from '../BuildFunction.js';
 
 import { createGlobalContextMock } from './utils/context.js';
 import { getErrorAsync } from './utils/error.js';
@@ -56,7 +56,7 @@ describe(BuildWorkflowValidator, () => {
     expect(error.errors[0]).toBeInstanceOf(BuildConfigError);
     expect(error.errors[0].message).toBe('Duplicated step IDs: "test1", "test3"');
   });
-  test('input set to a non-allowed value', async () => {
+  test('input sdfet to a non-allowed value', async () => {
     const ctx = createGlobalContextMock();
 
     const id1 = 'test1';
@@ -235,10 +235,10 @@ describe(BuildWorkflowValidator, () => {
       'Input parameter "id6" for step "step_id" is set to "${ wrong.aaa }" which is not of type "number" or is not step or context reference.'
     );
     expect((error as BuildWorkflowError).errors[4].message).toBe(
-      'Input parameter "id7" for step "step_id" is set to "${ invalidFunction("foo") }" which is not a valid build-in function name.'
+      'Input parameter "id7" for step "step_id" is set to "${ invalidFunction("foo") }" which is not a valid built-in function name.'
     );
     expect((error as BuildWorkflowError).errors[5].message).toBe(
-      'Input parameter "id8" for step "step_id" contains syntax error in "${ hashFiles("foo) }".'
+      'Input parameter "id8" for step "step_id" contains an error in expression \'hashFiles("foo)\': Unclosed quote after "foo)" at character 15.'
     );
   });
   test('output from future step', async () => {
@@ -538,7 +538,7 @@ describe(BuildWorkflowValidator, () => {
     );
   });
 
-  test('non-existing  function module', async () => {
+  test('non-existing function module', async () => {
     const ctx = createGlobalContextMock({ runtimePlatform: BuildRuntimePlatform.LINUX });
     const workflow = new BuildWorkflow(ctx, {
       buildSteps: [],

packages/steps/src/__tests__/fixtures/build.yml

@@ -33,4 +33,4 @@ build:
         inputs:
           key: cache-key
           paths:
-            - src
+            - src

packages/steps/src/cli/cli.ts

@@ -23,9 +23,7 @@ export class CliContextProvider implements ExternalBuildContextProvider {
     public readonly projectSourceDirectory: string,
     public readonly projectTargetDirectory: string,
     public readonly defaultWorkingDirectory: string,
-    public readonly buildLogsDirectory: string,
-    public readonly buildDirectory: string,
-    public readonly projectRootDirectory: string
+    public readonly buildLogsDirectory: string
   ) {}
   public get env(): BuildStepEnv {
     return this._env;
@@ -54,8 +52,6 @@ async function runAsync(
       relativeProjectDirectory,
       relativeProjectDirectory,
       relativeProjectDirectory,
-      relativeProjectDirectory,
-      relativeProjectDirectory,
       relativeProjectDirectory
     ),
     false