Repository: steveukx/git-js
Pull request #664 (2021-08-12 17:38:37)

feat: task callback types defined as single function type
The TypeScript `Parameters` type doesn't permit creating a type that is the discriminating union of the argument tuples, meaning the `SimpleGitTaskCallback` was always typed as the error state `(err: E) => void`.

To resolve this, the `SimpleGitTaskCallback` is now typed with a single function type in the same way as the official `node` types: `(err: null | E, data: T) => void`.


Patch:
diff --git a/src/lib/task-callback.ts b/src/lib/task-callback.ts
index fbe3116..1b5c9a3 100644
--- a/src/lib/task-callback.ts
+++ b/src/lib/task-callback.ts
@@ -11,10 +11,7 @@ export function taskCallback<R>(task: SimpleGitTask<R>, response: Promise<R>, ca
 
    const onError = (err: GitError | GitResponseError) => {
       if (err?.task === task) {
-         if (err instanceof GitResponseError) {
-            return callback(addDeprecationNoticeToError(err));
-         }
-         callback(err);
+         callback((err instanceof GitResponseError) ? addDeprecationNoticeToError(err) : err, undefined as any);
       }
    };
 
diff --git a/src/lib/types/handlers.ts b/src/lib/types/handlers.ts
index 2fc10f5..3edf508 100644
--- a/src/lib/types/handlers.ts
+++ b/src/lib/types/handlers.ts
@@ -4,11 +4,7 @@ import { GitError } from '../errors/git-error';
  * The node-style callback to a task accepts either two arguments with the first as a null
  * and the second as the data, or just one argument which is an error.
  */
-export interface SimpleGitTaskCallback<T = string, E extends GitError = GitError> {
-   (err: null, data: T): void;
-
-   (err: E): void;
-}
+export type SimpleGitTaskCallback<T = string, E extends GitError = GitError> = (err: E | null, data: T) => void;
 
 /**
  * The event data emitted to the progress handler whenever progress detail is received.


Test patch:
diff --git a/test/integration/change-directory.spec.ts b/test/integration/change-directory.spec.ts
index cb11185..fb31ffe 100644
--- a/test/integration/change-directory.spec.ts
+++ b/test/integration/change-directory.spec.ts
@@ -77,7 +77,7 @@ describe('change-directory', () => {
       await wait(250);
 
       expect(spies[0]).toHaveBeenCalledWith(null, goodDir);
-      expect(spies[1]).toHaveBeenCalledWith(expect.any(Error));
+      expect(spies[1]).toHaveBeenCalledWith(expect.any(Error), undefined);
       expect(spies[2]).not.toHaveBeenCalled();
 
    });
diff --git a/test/unit/child-process.spec.ts b/test/unit/child-process.spec.ts
index b9e0262..6a5b979 100644
--- a/test/unit/child-process.spec.ts
+++ b/test/unit/child-process.spec.ts
@@ -22,7 +22,7 @@ describe('child-process', () => {
       await closeWithError('SOME ERROR');
 
       const error = await promiseError(queue);
-      expect(callback).toHaveBeenCalledWith(error);
+      expect(callback).toHaveBeenCalledWith(error, undefined);
       assertGitError(error, 'SOME ERROR');
    });
 
diff --git a/test/unit/cwd.spec.ts b/test/unit/cwd.spec.ts
index e7d4124..87e0041 100644
--- a/test/unit/cwd.spec.ts
+++ b/test/unit/cwd.spec.ts
@@ -29,7 +29,7 @@ describe('cwd', () => {
       git.cwd('./invalid_path', callback);
 
       await wait();
-      expect(callback).toHaveBeenCalledWith(expect.any(Error),);
+      expect(callback).toHaveBeenCalledWith(expect.any(Error), undefined);
       assertNoExecutedTasks();
    });
 
diff --git a/test/unit/merge.spec.ts b/test/unit/merge.spec.ts
index 696ff7d..798152b 100644
--- a/test/unit/merge.spec.ts
+++ b/test/unit/merge.spec.ts
@@ -64,7 +64,7 @@ describe('merge', () => {
 
          await closeWithError(message, 128);
          await wait();
-         expect(later).toHaveBeenCalledWith(like({message}));
+         expect(later).toHaveBeenCalledWith(like({message}), undefined);
 
       });
 
diff --git a/test/unit/raw.spec.ts b/test/unit/raw.spec.ts
index a5ce08c..18d1ede 100644
--- a/test/unit/raw.spec.ts
+++ b/test/unit/raw.spec.ts
@@ -31,7 +31,7 @@ describe('raw', () => {
       const task = git.raw([], callback);
       const error = await promiseError(task);
 
-      expect(callback).toHaveBeenCalledWith(error);
+      expect(callback).toHaveBeenCalledWith(error, undefined);
       assertGitError(error, 'Raw: must supply one or more command to execute');
       assertNoExecutedTasks();
    });
@@ -40,7 +40,7 @@ describe('raw', () => {
       const task = git.raw(callback as any);
       const error = await promiseError(task);
 
-      expect(callback).toHaveBeenCalledWith(error);
+      expect(callback).toHaveBeenCalledWith(error, undefined);
       assertGitError(error, 'must supply one or more command');
       assertNoExecutedTasks();
    });
@@ -94,4 +94,16 @@ describe('raw', () => {
       assertExecutedCommands('some', 'thing');
    });
 
+   it('accepts array arg: callback', async () => {
+      let called = false;
+      const queue = git.raw(['some', 'thing'], (err, data) => {
+         expect(err).toBe(null);
+         expect(data).toBe('result');
+         called = true;
+      });
+
+      await closeWithSuccess('result');
+      await queue;
+      expect(called).toBe(true);
+   });
 })
diff --git a/test/unit/status.spec.ts b/test/unit/status.spec.ts
index e4675e7..37f7535 100644
--- a/test/unit/status.spec.ts
+++ b/test/unit/status.spec.ts
@@ -117,7 +117,7 @@ describe('status', () => {
          const queue = git.status(callback);
          await closeWithError('unknown');
 
-         expect(callback).toBeCalledWith(await promiseError(queue));
+         expect(callback).toBeCalledWith(await promiseError(queue), undefined);
          assertExecutedCommands(...statusCommands());
       });