Introduce typescript types (possibly via conversion to typescript)

[TimothyJones]

The original repository had types through DefinitelyTyped here.

Forking means the types won't have come through. We could:

1. Copy the definitely-typed types to this repo
2. Upload a copy to definitely-typed/commit-and-tag-version
3. Convert this repo to typescript

I think the smallest effort would be option 1. Long term, conversion to typescript would be ideal, because then we'd be able to merge #21, #19, #18 and #16, all of which require ESM.

[joh-klein]

Quick note (as you are probably aware): the types from DefinitelyTyped are pretty out of date (they are for v7.0.0)

[Mitiryl]

Hi @TimothyJones ,
any updates about this?

[TimothyJones]

I haven't had the chance to look at this one in detail, sorry. I think it needs some close attention. I would happily accept a PR if you have some spare capacity.

If not, even just looking at the definitely-typed types to see what needs updating would be helpful.

[helmturner]

@TimothyJones I've started some work on option 3. What do you think about the balance between breaking changes and correctness? Are we thinking major version upgrade? For instance, there may be some libs that need swapping for the most correct types.

The first I've encountered is yargs. There are definitely-typed typings available, but they're not great. The typings for commander are much better with the @commander/extra-typings library (maybe not the best example, since I think I can make the swap without introducing breaking changes).

Also, it looks like commander has taken the lead as the preferred CLI lib since 2021. Can you think of any objections to the swap (and associated refactoring) in this particular instance, or general guidelines for other dependency updates I might encounter?

Edit: Also, do you know what the status is on deprecating the --message option? This is the --help output:
[DEPRECATED] Commit message, replaces %s with new version.\nThis option will be removed in the next major version, please use --releaseCommitMessageFormat.

[TimothyJones]

It's late here, so I'll be brief, but since you're actively working on this, I wanted to reply quickly.

I've started some work on option 3.

Firstly: <3 Thank you! This is amazing. If you have it on a branch somewhere, I could lend a hand / take a look etc - if you want.

In general I was hoping to do this without breaking changes - as in, changing the external behaviour or API.

I have a general philosophy of doing one thing at once, so as to keep the risk (and workload) down. However, if you think it's not possible to avoid bringing in several changes, then feel free to use your judgement.

(Aside: the original author for this project was also the author of yargs (at least, I think I have that right), so I would imagine it was chosen due to familiarity and preference)

It would be good to redo the way that options are handled, as at the moment there isn't consistency in the options available via CLI or config file. There's some context here and on the linked issues from there. I don't mind replacing yargs, but it would be great to use whatever is best for supporting fixes for those issues.

My gut feel is that it might be more practical and safer to force the typings for yargs with as, any etc (as long as we can keep that constrained to one or two files) in order to get Typescript going, and then do the refactor to commander (or whatever) after.

or general guidelines for other dependency updates I might encounter?

I have two main concerns - firstly, I'd like to prioritise maintainability - part of the reason that I'd like to bring typescript in is to reduce the risk of small changes introducing big mistakes (since this project is pretty stable, it's likely to receive more small changes than big, in general - the safer we can make those, the better).

Secondly, a big refactor to typescript is going to touch both the tests and the implementation - and changing both together is pretty risky. I think it would be best to avoid changing too much of the implementation at the same time as we touch the tests.

It would be great to have a point in the repo where we can say "here's a working version where we brought in typescript" and then one where we can say "and here's where we swapped to better dependencies for typescript" - even if there's no release between the two. I like this as the path-of-least risk, but of course there may be some reason it's not practical to do it that way.

Update: I was not brief 😂

[helmturner]

Haha! Thanks for the quick and thoughtful response, though!

Breaking Changes

I think you're right about trying to isolate changes to the type layer. Ultimately, may be impossible to fully type everything without run-time changes, so on the first pass I'll probably have to use some anys where there's ambiguity that a type-cast can't solve.

My approach will be to get to 100% type coverage without touching any test files, ensuring run-time semantics are consistent. I'll file issues (or a super-issue?) to track any unsafe type compromises that must be made.

Options

As for #31, that will necessarily involve breaking changes. I realized this as my initial optimism about switching to commander without breaking changes began to fade when I got to types option, which is an array of objects.

Commander doesn't account for object-typed values at all, but objects can be worked around with dot notation. Unfortunately, an array of objects is off the table without some pretty funky CLI syntax.

That said, I've already done a little playing around with the swap... would it be a good idea to close #31 with a breaking change before the TS refactor?

Roadmap

It seems to me that introducing TS to popular JS libs increases their popularity. Closing #31 means a major API change for both the CLI and the JSON config. It would make commit-and-tag-version no longer backward-compatible with scripts and config files for standard-version or any version of commit-and-tag-version prior to the breaking change.

If that's on the roadmap, I would argue that making such a change now would have less downstream impact than doing so after this fork sees wider adoption.

Edit: On second thought...

Maybe I can fix #31 without breaking changes by implementing commander with:

• dot-notation for existing object-valued CLI options (and maybe a transform)
• diverging syntax for not-currently-available CLI options
• a transform on JSON options to normalize them to the types received by CLI args

Not 100% sure

Edit edit: Nah, not that easy.

[TimothyJones]

I'll file issues (or a super-issue?) to track any unsafe type compromises that must be made.

Please feel free to do so!

Commander doesn't account for object-typed values at all,

For some things, it's hard to do via a CLI without resorting to json (which is pretty unpleasant in a CLI). I don't mind if those need to be specified by config - if it's going to be clumsy to specify via CLI, then a hybrid approach would be the most ergonomic for users. One drawback of the current implementation is that it doesn't always support combining options in config and CLI (see #28)

That said, I've already done a little playing around with the swap... would it be a good idea to close #31 with a breaking change before the TS refactor?

Yes, I think this makes a great deal of sense.

It seems to me that introducing TS to popular JS libs increases their popularity.

This is another good reason :) As far as I can see from searching npm, this is the most popular fork.

Closing #31 means a major API change for both the CLI and the JSON config.

Perhaps I'm missing something, but why would it be a breaking change for the JSON config?

a transform on JSON options to normalize them to the types received by CLI args

Ah! So, I had assumed that whatever format the CLI library outputs would be irrelevant from the perspective of the rest of the code. As in, we would bring options in from (potentially) a few different sources, and populate a config object. This object would probably the same as the format for the JSON config (so there isn't need for another mental model).

This approach would also mean that only one file would have to change if we ever wanted to replace the CLI args library again - and it would keep the reading of the package.json and any version.rc files / future alternative config sources independent.

It would look something like:

// Assuming combineConfig(newConfig, currentConfig) => Config

const config = combineConfig(
  readCliConfig(),
  combineConfig(readPackageConfig(), defaultConfig)
);

It would make commit-and-tag-version no longer backward-compatible with scripts and config files for standard-version or any version of commit-and-tag-version prior to the breaking change.

I don't mind making breaking changes if there's a clear reason to do so. And, we might be able to get pretty close for most common options.

I prefer releasing minor well-documented breaking changes, over bending the API out of shape to avoid making them. I've already released breaking changes since forking, and according to the npm stats, the current major release is the most popular, so people are comfortable migrating.

Edit: Maybe I can fix [...]
Edit edit: Nah, not that easy.

This is very relateable 😎

[SharpSeeEr]

I am a huge fan of Typescript and would love to see this get converted. Has there been any progress made? I would love to help.

[helmturner]

I believe we came to the conclusion that we would consider #31 to be a blocker to this (see also: #28 for context). I spun up a codespace that has some playground work, but I was hoping for a bit more feedback before just going cowboy with the API 😆

Edit: I suppose there's no reason that fringe modules can't begin conversion, I just always find it easier to start from the entrypoint and work backwards.

[SharpSeeEr]

I don't see #31 as a blocker. It's a little more complex, but definitely possible. I'll look into it and see what I can figure out.

[helmturner]

💪🏽💪🏽
If you start making some progress and want to throw up a branch I'm happy to help divide and conquer. TS would be a game changer 🙌

[D4RKAR117]

Just throwing around this but I'm working on a future pr over a fork, its pretty early but anyone interested to throw suggestions that can benefit the "refactor" watch this