Skip to content

fix: Warn when defaulting to --inc=patch in CLI#859

Merged
owlstronaut merged 1 commit intonpm:mainfrom
pjohnmeyer:fix/add-warning-invalid-cli-increment
May 7, 2026
Merged

fix: Warn when defaulting to --inc=patch in CLI#859
owlstronaut merged 1 commit intonpm:mainfrom
pjohnmeyer:fix/add-warning-invalid-cli-increment

Conversation

@pjohnmeyer
Copy link
Copy Markdown
Contributor

Adds a warning log message to the CLI when the --increment option is provided with an invalid increment type. This provides adequate feedback to the user about what is happening, and prepares them for a future where this may no longer be allowed.

In #108 (comment), @mklement0 points out (correctly, IMO):

Quietly accepting invalid input is problematic.

As @lukekarrys points out in his reply, however, he considers erroring here to be a breaking change. I would tend to agree as this behavior has become part of the contract between the CLI user and the CLI.

The issue is being tracked for the next major version of the CLI, so I offer up that we can safely add the warning indicating that a bad value has been provided, simultaneously preserving the contract, and preparing users for the possibility of a future change. As console.warn does not output to stdout, existing redirection and/or piping of output should survive this change, thus it will not be breaking. Examples:

% ./bin/semver.js --inc patchouli 3.2.1 > ./new-version.redirected
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
% cat ./new-version.redirected
3.2.2

% ./bin/semver.js --inc patchouli 3.2.1 | tee ./new-version.teed
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
3.2.2
% cat ./new-version.teed 
3.2.2

The only breakage, then, would occur if we consider stderr (or in this case lack thereof) output to also be part of the CLI contract. Example:

% ./bin/semver.js --inc patchouli 3.2.1 &> ./new-version.err    
% cat ./new-version.err 
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
3.2.2

In this case the warning and the new version are both output to ./new-version.err, and any processing of that output expecting only 3.2.2 might fail.

Alternative

We could, instead, add a flag to the CLI to opt-in to new behavior re: the --inc flag. This could be done in three different ways I can immediately think of:

  • --increment-strict: an alternate flag used instead of --increment that enforces the validity of the argument; downside is that we would then want to remove this in the next release, creating another breaking change
  • --future: a generic flag that hides all breaking "next version" (currently 8) functionality behind a feature flag, used in combination with --increment to enforce the validity of the argument
  • --verbose: adds the additional "warning" logging added in this PR, with the opportunity for more usage elsewhere

I think the approach in this PR is the simplest and requires the least change in the future, but --future and --verbose open up other options going forward.

References

Relates to #108

@pjohnmeyer pjohnmeyer requested a review from a team as a code owner May 7, 2026 13:53
Comment thread bin/semver.js
@owlstronaut owlstronaut merged commit 3905343 into npm:main May 7, 2026
25 of 29 checks passed
@github-actions github-actions Bot mentioned this pull request Mar 20, 2026
owlstronaut pushed a commit that referenced this pull request May 8, 2026
🤖 I have created a release *beep* *boop*
---


## [7.8.0](v7.7.4...v7.8.0)
(2026-05-08)
### Features
*
[`0d0a0a2`](0d0a0a2)
[#855](#855) Add `truncate`
function (#855) (@pjohnmeyer, @owlstronaut)
### Bug Fixes
*
[`3905343`](3905343)
[#859](#859) Warn when defaulting
to --inc=patch in CLI (@pjohnmeyer)
### Documentation
*
[`c368af6`](c368af6)
[#853](#853) fix typos in
documentation (#853) (@ankitkumar572005)
*
[`37776c3`](37776c3)
[#846](#846) fix BNF grammar to
distinguish prerelease from build identifiers (#846) (@abhu85, @claude)
### Chores
*
[`9542e09`](9542e09)
[#860](#860) template-oss-apply
(@owlstronaut)
*
[`937bc2c`](937bc2c)
[#860](#860)
`template-oss-apply@5.0.0` (@owlstronaut)
*
[`6946fef`](6946fef)
[#852](#852) bump
@npmcli/template-oss from 4.29.0 to 4.30.0 (#852) (@dependabot[bot],
@npm-cli-bot)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants