Deprecate version retrieval #1954
Conversation
I am totally in favor of deprecating it and removing in the next major release, which should hardly cause any migration complications because the feature has never been documented and is not very useful anyway, so unlikely to be relied on. The fact that the version is included in the object returned from (By the way, still waiting for your reply in #1921. No hurry though, just reminding in case you forgot which I myself definitely would've considering the issue has been closed for a while now.)
As far as I am concerned, we could just leave it. Doesn't harm, doesn't imply any complications for the implementation. Gives a meaning to an otherwise meaningless call. (A different meaning could be to delete the version option, but I don't think it is something you want to support because the fact you closed #1933 indicates that you don't want to make modifications to version options possible.) |
Apropos meaningless calls, I still think the change from #1933 that enables updates to the version number should be accepted. All the other changes suggested there are not really important because they are just reinventions of the existing behavior of But this particular change would give meaning to repeated PS: I am referring to this use case scenario from #1933: var program = new Command();
program.version('1.0.0');
if ('EARLY_ADOPTER' in process.env) {
program.version('2.0.0'); // expectation: the version number is simply updated
}
program.outputHelp(); // reality: contains version option twice if early adopter
program.parse(['--version'], { from: 'user' }); // output is always '1.0.0'It is unsettling someone could write code like this unknowingly and get completely unexpected results. |
|
An argument in defense of version retrieval via |
I did wonder about that use case. However, not particularly useful currently as only defined on the program and not on subcommands. |
In |
|
The code search is getting useful! Somewhat annoyingly, I found an exercise saying to retrieve the version with I'll try and get over my current annoyance... 😆 |
Pull Request
Problem
An early implementation of
.version()acted a bit like a getter/setter for a version property:For historical interest, in that code
this._versionalso got used to decide whether to guess the version when.parse()was called! Long gone.When
.opts()got added, the version option/property got returned as well. [sic]Retrieving the version property has never been documented, and the JSDoc and TypeScript typings define the version string parameter as required (so do not include the getter usage).
I definitely don't think the version should be returned in
.opts()(and that is only done when.storeOptionsAsProperties()is turned on, so already a legacy behaviour).I am dubious about supporting a getter for the version. I think of
.version()as a convenience for adding the version option, and not as a getter for a version property. Because it has never been documented, I have never used it that way myself and don't recall any mention of it in issues. However, I feel less strongly about this than (1) and don't mind keeping it if people are using it.Solution
Added to Deprecated documentation to make it explicit that may remove the functionality in future.
ChangeLog
cmd.version()with no parameterscmd.opts().version(and.storeOptionsAsProperties())