fix: use || instead of ?? for CLI flag override precedence in prepars… #2576
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…e hook
The preparse hook was using ?? (nullish coalescing) instead of || (logical OR) when checking if a CLI flag was already present. This caused a bug where CLI arguments would not override flags-dir values in certain scenarios.
Bug introduced in: c83f81a (2025-04-22, SF CLI 2.100.0) The change added a necessary guard (flagOptions.char &&) to prevent checking for undefined short chars, but incorrectly used ?? instead of || to chain the conditions. Since ?? only evaluates the right side when left is null/undefined, and false is neither, the subsequent checks were skipped.
Affected scenarios (5 of 14 systematic test cases):
Example: Flag defined as Flags.string({ char: 'l' }) with flags-dir containing a 'test-level' file. Running
sf cmd --test-level NoTestRunshould override the file value, but with ?? it was erroneously combined (conflict error).This fix restores the documented behavior from PR #1536: "Flags/values provided by the user will override any flag/values found by --flags-dir -- unless the flag supports
multiple, in which case they will all be combined."The eslint-disable is intentional because the lint rule incorrectly suggests ?? when || is actually required for proper boolean short-circuit evaluation.
What does this PR do?
Describe the feature with use cases or bug details. Interesting/complex design choices or weird workarounds should be documented in the code but can be included here instead if there is a valid reason it shouldn't be in the code.
Acceptance Criteria
How do you know this change is successful? What is the scope of this change? What tests test the criteria (or why no tests)?
The Acceptance Criteria can be copied from the work item if they exist there but it is useful to have on the PR when reviewing the code.
Testing Notes
Anything additional to note about testing this PR. How did you test it? Special setup? Additional manual test cases? Things not tested?
Checklist
What issues does this PR fix or reference?
@W-20991305@