Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript: git.reset() doesn't allow string arg for reset mode #575

Closed
ockham opened this issue Feb 12, 2021 · 4 comments
Closed

TypeScript: git.reset() doesn't allow string arg for reset mode #575

ockham opened this issue Feb 12, 2021 · 4 comments
Labels
more-info-needed More information is required in order to investigate

Comments

@ockham
Copy link

ockham commented Feb 12, 2021

When attempting to compile a script that uses simple-git with TypeScript, I'm getting the following error for a line that contains await simpleGit.reset( 'hard' ):

bin/plugin/lib/git.js:108:25 - error TS2769: No overload matches this call.
  Overload 1 of 3, '(mode: ResetMode, options?: string[] | ResetOptions | undefined, callback?: SimpleGitTaskCallback<string, GitError> | undefined): Response<...>', gave the following error.
    Argument of type '"hard"' is not assignable to parameter of type 'ResetMode'.
  Overload 2 of 3, '(mode: ResetMode, callback?: SimpleGitTaskCallback<string, GitError> | undefined): Response<string>', gave the following error.
    Argument of type '"hard"' is not assignable to parameter of type 'ResetMode'.
  Overload 3 of 3, '(options?: string[] | ResetOptions | undefined, callback?: SimpleGitTaskCallback<string, GitError> | undefined): Response<string>', gave the following error.
    Argument of type '"hard"' is not assignable to parameter of type 'string[] | ResetOptions | undefined'.

108  await simpleGit.reset( 'hard' );

Reading typedefs and unit tests, it seems like I need a type assertion to ResetMode:

await simpleGit.reset( 'hard' as ResetMode );

However, this isn't clearly reflected in the docs:

[the argument] sets the reset mode to one of the supported types (use a constant from the exported ResetMode enum, or a string equivalent: mixed, soft, hard, merge, keep)

(Highlight mine.)

I'm not familiar enough yet with TypeScript, but is there any way to allow passing these strings directly (without the type assertion)? Otherwise, it'd be great to document that it's required when using TS.

@steveukx
Copy link
Owner

Hi, when using TypeScript it is currently necessary to either use the ResetMode enum, or cast the string with as ResetMode in order to be recognised as a valid reset mode - examples of both can be found in the unit tests. For JavaScript you only need supply the string equivalent to one of the ResetMode values.

If you would prefer to use a string without type information, you can use an array of strings (overload 3 in your issue detail) to pass any supported strings to the underlying git reset command:

// to use the enum directly
import simpleGit, {ResetMode} from 'simple-git';
simpleGit().reset(ResetMode.HARD);
// to use strings without type information
import simpleGit, {ResetMode} from 'simple-git';
simpleGit().reset(['--hard']);
// to use javascript
const simpleGit = require('simple-git');

// with a string literal
simpleGit().reset('hard');

// or with the constant
simpleGit().reset(simpleGit.ResetMode.HARD);

@steveukx
Copy link
Owner

From your downstream PR, it looks as though lib/git.js would work fine with the updated version of simple-git - you just need to change the import from require('simple-git/promise') to be require('simple-git') as all previously chained commands are now also async awaitable.

@steveukx steveukx added the more-info-needed More information is required in order to investigate label Feb 12, 2021
@ockham
Copy link
Author

ockham commented Feb 12, 2021

From your downstream PR, it looks as though lib/git.js would work fine with the updated version of simple-git - you just need to change the import from require('simple-git/promise') to be require('simple-git') as all previously chained commands are now also async awaitable.

Thanks for looking! We've made that change, it's it's working nicely 👍 (It's really just the TypeScript ignore I was hoping to maybe get rid of 😅 -- but it's of course not so bad to keep it around a little longer.)

@no-response no-response bot removed the more-info-needed More information is required in order to investigate label Feb 12, 2021
@steveukx steveukx added the more-info-needed More information is required in order to investigate label Feb 22, 2021
@no-response
Copy link

no-response bot commented Mar 15, 2021

This issue has been automatically closed due to a lack of response. If your problem persists please open a new issue including any additional detail requested from this issue. For more detail on the issues/pull requests see ISSUES_AND_PULL_REQUESTS

@no-response no-response bot closed this as completed Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed More information is required in order to investigate
Projects
None yet
Development

No branches or pull requests

2 participants