Refactor withGit helper: move to end of file and make callback a positional parameter

Author: katiebauchman

packages/cli-kit/src/public/node/git.ts

@@ -27,12 +27,9 @@ export async function initializeGitRepository(directory: string, initialBranch =
   outputDebug(outputContent`Initializing git repository at ${outputToken.path(directory)}...`)
   await ensureGitIsPresentOrAbort()
   // We use init and checkout instead of `init --initial-branch` because the latter is only supported in git 2.28+
-  await withGit({
-    directory,
-    callback: async (repo) => {
-      await repo.init()
-      await repo.checkoutLocalBranch(initialBranch)
-    },
+  await withGit({directory}, async (repo) => {
+    await repo.init()
+    await repo.checkoutLocalBranch(initialBranch)
   })
 }
 
@@ -46,7 +43,7 @@ export async function initializeGitRepository(directory: string, initialBranch =
  * @returns Files ignored by the lockfile.
  */
 export async function checkIfIgnoredInGitRepository(directory: string, files: string[]): Promise<string[]> {
-  return withGit({directory, callback: (repo) => repo.checkIgnore(files)})
+  return withGit({directory}, (repo) => repo.checkIgnore(files))
 }
 
 export interface GitIgnoreTemplate {
@@ -197,12 +194,9 @@ export async function downloadGitRepository(cloneOptions: GitCloneOptions): Prom
       await git(simpleGitOptions).clone(repository!, destination, options)
 
       if (latestTag) {
-        await withGit({
-          directory: destination,
-          callback: async (localGitRepository) => {
-            const latestTag = await getLocalLatestTag(localGitRepository, repoUrl)
-            await localGitRepository.checkout(latestTag)
-          },
+        await withGit({directory: destination}, async (localGitRepository) => {
+          const latestTag = await getLocalLatestTag(localGitRepository, repoUrl)
+          await localGitRepository.checkout(latestTag)
         })
       }
     } catch (err) {
@@ -240,10 +234,7 @@ async function getLocalLatestTag(repository: SimpleGit, repoUrl: string): Promis
  * @returns The latest commit of the repository.
  */
 export async function getLatestGitCommit(directory?: string): Promise<DefaultLogFields & ListLogLine> {
-  const logs = await withGit({
-    directory,
-    callback: (repo) => repo.log({maxCount: 1}),
-  })
+  const logs = await withGit({directory}, (repo) => repo.log({maxCount: 1}))
   if (!logs.latest) {
     throw new AbortError(
       'Must have at least one commit to run command',
@@ -262,7 +253,7 @@ export async function getLatestGitCommit(directory?: string): Promise<DefaultLog
  * @returns A promise that resolves when the files are added to the index.
  */
 export async function addAllToGitFromDirectory(directory?: string): Promise<void> {
-  await withGit({directory, callback: (repo) => repo.raw('add', '--all')})
+  await withGit({directory}, (repo) => repo.raw('add', '--all'))
 }
 
 export interface CreateGitCommitOptions {
@@ -279,10 +270,7 @@ export interface CreateGitCommitOptions {
  */
 export async function createGitCommit(message: string, options?: CreateGitCommitOptions): Promise<string> {
   const commitOptions = options?.author ? {'--author': options.author} : undefined
-  const result = await withGit({
-    directory: options?.directory,
-    callback: (repo) => repo.commit(message, commitOptions),
-  })
+  const result = await withGit({directory: options?.directory}, (repo) => repo.commit(message, commitOptions))
   return result.commit
 }
 
@@ -293,7 +281,7 @@ export async function createGitCommit(message: string, options?: CreateGitCommit
  * @returns The HEAD symbolic reference of the repository.
  */
 export async function getHeadSymbolicRef(directory?: string): Promise<string> {
-  const ref = await withGit({directory, callback: (repo) => repo.raw('symbolic-ref', '-q', 'HEAD')})
+  const ref = await withGit({directory}, (repo) => repo.raw('symbolic-ref', '-q', 'HEAD'))
   if (!ref) {
     throw new AbortError(
       "Git HEAD can't be detached to run command",
@@ -346,7 +334,7 @@ export async function ensureInsideGitDirectory(directory?: string): Promise<void
  * @returns True if the directory is inside a .git directory tree.
  */
 export async function insideGitDirectory(directory?: string): Promise<boolean> {
-  return withGit({directory, callback: (repo) => repo.checkIsRepo()})
+  return withGit({directory}, (repo) => repo.checkIsRepo())
 }
 
 export class GitDirectoryNotCleanError extends AbortError {}
@@ -369,29 +357,7 @@ export async function ensureIsClean(directory?: string): Promise<void> {
  * @returns True is the .git directory is clean.
  */
 export async function isClean(directory?: string): Promise<boolean> {
-  return (await withGit({directory, callback: (git: SimpleGit) => git.status()})).isClean()
-}
-
-async function withGit<T>({
-  directory,
-  callback,
-}: {
-  directory?: string
-  callback: (git: SimpleGit) => Promise<T>
-}): Promise<T> {
-  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-  // @ts-ignore
-  const repo = git({baseDir: directory})
-  try {
-    return await callback(repo)
-  } catch (err) {
-    if (err instanceof Error) {
-      const abortError = new AbortError(err.message)
-      abortError.stack = err.stack
-      throw abortError
-    }
-    throw err
-  }
+  return (await withGit({directory}, (git: SimpleGit) => git.status())).isClean()
 }
 
 /**
@@ -401,7 +367,7 @@ async function withGit<T>({
  * @returns String with the latest tag or undefined if no tags are found.
  */
 export async function getLatestTag(directory?: string): Promise<string | undefined> {
-  const tags = await withGit({directory, callback: (repo) => repo.tags()})
+  const tags = await withGit({directory}, (repo) => repo.tags())
   return tags.latest
 }
 
@@ -416,19 +382,39 @@ export async function removeGitRemote(directory: string, remoteName = 'origin'):
   outputDebug(outputContent`Removing git remote ${remoteName} from ${outputToken.path(directory)}...`)
   await ensureGitIsPresentOrAbort()
 
-  await withGit({
-    directory,
-    callback: async (repo) => {
-      // Check if remote exists first
-      const remotes = await repo.getRemotes()
-      const remoteExists = remotes.some((remote: {name: string}) => remote.name === remoteName)
-
-      if (!remoteExists) {
-        outputDebug(outputContent`Remote ${remoteName} does not exist, no action needed`)
-        return
-      }
+  await withGit({directory}, async (repo) => {
+    // Check if remote exists first
+    const remotes = await repo.getRemotes()
+    const remoteExists = remotes.some((remote: {name: string}) => remote.name === remoteName)
 
-      await repo.removeRemote(remoteName)
-    },
+    if (!remoteExists) {
+      outputDebug(outputContent`Remote ${remoteName} does not exist, no action needed`)
+      return
+    }
+
+    await repo.removeRemote(remoteName)
   })
 }
+
+async function withGit<T>(
+  {
+    directory,
+  }: {
+    directory?: string
+  },
+  callback: (git: SimpleGit) => Promise<T>,
+): Promise<T> {
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+  // @ts-ignore
+  const repo = git({baseDir: directory})
+  try {
+    return await callback(repo)
+  } catch (err) {
+    if (err instanceof Error) {
+      const abortError = new AbortError(err.message)
+      abortError.stack = err.stack
+      throw abortError
+    }
+    throw err
+  }
+}