Only use the last value when parsing duplicate cli arguments #19416
Conversation
WalkthroughThe CLI argument parser was modified to collapse multi-valued flag arrays to their last element before handling the default marker Pre-merge checks✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/@tailwindcss-cli/src/utils/args.test.ts (1)
20-34: Duplicate-argument test looks good; consider adding a'-'caseThis test correctly locks in the “last value wins” behavior for repeated flags and matches the new normalization logic in
args().Given the special handling of
'-'via the__IO_DEFAULT_VALUE__sentinel, consider adding a companion test where the final duplicate is'-'to protect that behavior:it('should respect "-" when it is the last duplicate value', () => { expect( args( { '--output': { type: 'string', description: 'Output file' }, }, ['--output', 'output.css', '--output', '-'], ), ).toMatchInlineSnapshot(` { "--output": "-", "_": [], } `) })This would catch regressions where
'-'stops being normalized correctly in the duplicate-flag scenario.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/@tailwindcss-cli/src/utils/args.test.ts(1 hunks)packages/@tailwindcss-cli/src/utils/args.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@tailwindcss-cli/src/utils/args.test.ts (1)
packages/@tailwindcss-cli/src/utils/args.ts (1)
args(70-116)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/@tailwindcss-cli/src/utils/args.ts (1)
79-91: Excellent fix! The ordering now correctly handles the sentinel conversion.The refactored logic properly addresses the issue from the past review comment:
- Arrays are collapsed to their last element first (lines 82-84)
- The sentinel
__IO_DEFAULT_VALUE__is converted to-afterward (lines 86-88)This ensures that when
-is the last duplicate value (e.g.,--output output.css --output -), it's correctly preserved as-in the final result.Optional: Consider a non-mutating approach for array access.
While
.pop()works correctly here, it mutates the array. A more idiomatic approach would be:- if (key !== '_' && Array.isArray(value)) { - value = value.pop() - } + if (key !== '_' && Array.isArray(value)) { + value = value[value.length - 1] + }This is purely stylistic since the mutation doesn't affect the final result (we reassign
parsed[key]anyway). The non-mutating approach is slightly clearer about intent: "get the last element" vs "remove and get the last element."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/@tailwindcss-cli/src/utils/args.test.ts(1 hunks)packages/@tailwindcss-cli/src/utils/args.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@tailwindcss-cli/src/utils/args.test.ts (1)
packages/@tailwindcss-cli/src/utils/args.ts (1)
args(70-122)
🔇 Additional comments (2)
packages/@tailwindcss-cli/src/utils/args.test.ts (2)
20-34: LGTM! Good test coverage for duplicate argument handling.This test correctly verifies that when the same flag is provided multiple times, the last value wins.
36-50: Excellent test for the sentinel value edge case.This test properly verifies the fix for the ordering issue mentioned in the past review comment. When
-(which becomes__IO_DEFAULT_VALUE__internally) appears as the last duplicate value, it should be correctly converted back to-after the array is collapsed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
25-25: Clarify “duplicate CLI arguments” behavior (spell out last‑wins).Explicitly stating “last value wins” reduces ambiguity and helps users understand what changed at a glance.
- - Correctly handle duplicate CLI arguments ([#19416](https://github.com/tailwindlabs/tailwindcss/pull/19416)) + - Duplicate CLI arguments now use the last value (last‑wins), matching v3 ([#19416](https://github.com/tailwindlabs/tailwindcss/pull/19416))
|
Thanks! |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
3 participants
Summary
When parsing cli args, 'mri' returns an array for duplicate args, which causes unexpected results. All current arguments assume single values only. Tailwind v3 works the same way.
Test plan
Updated tests.