feat: RFC 3339 duration Validator - #1554
Conversation
Closes open-circle#1497. Add an `isoDuration(message?)` action that validates a string is an ISO 8601 / RFC 3339 duration (`P[n]Y[n]M[n]W[n]DT[n]H[n]M[n]S`, e.g. `P1D`, `PT5H30M`, `P1Y2M3DT4H5M6S`). Decimal values are supported. A bare `P` or `PT` (no components) is rejected via a negative lookahead.
WalkthroughAdds an ISO 8601 / RFC 3339 duration validation action with a regular expression, typed issue and action interfaces, overloads, configurable messages, and synchronous validation. Invalid typed values produce duration issues. The new action and its public types are re-exported through the local module index and the main actions entrypoint. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
library/src/actions/index.ts (1)
42-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMaintain alphabetical sorting for exports.
For consistency with the rest of the barrel file, consider moving the
isoDurationexport to immediately followisoDateTimeSecond.♻️ Proposed refactor
-export * from './isoDuration/index.ts'; export * from './isoDateTime/index.ts'; export * from './isoDateTimeSecond/index.ts'; +export * from './isoDuration/index.ts';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@library/src/actions/index.ts` around lines 42 - 44, Reorder the exports in the barrel file so isoDateTime and isoDateTimeSecond remain in alphabetical order, followed immediately by isoDuration. Keep the export statements unchanged apart from their ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@library/src/actions/isoDuration/isoDuration.ts`:
- Around line 13-14: Update ISO_DURATION_REGEX so the optional time group’s T
designator requires a following digit via a positive lookahead, preventing
values such as P1YT from matching while preserving valid hour, minute, and
second components.
---
Nitpick comments:
In `@library/src/actions/index.ts`:
- Around line 42-44: Reorder the exports in the barrel file so isoDateTime and
isoDateTimeSecond remain in alphabetical order, followed immediately by
isoDuration. Keep the export statements unchanged apart from their ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b123ec4-dee7-460f-89cb-69366bfbcd33
📒 Files selected for processing (3)
library/src/actions/index.tslibrary/src/actions/isoDuration/index.tslibrary/src/actions/isoDuration/isoDuration.ts
| const ISO_DURATION_REGEX: RegExp = | ||
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent the T designator from matching when time components are absent.
In the current ISO_DURATION_REGEX, the time components (H, M, S) are fully optional inside the T group (?:T(?:...H)?(?:...M)?(?:...S)?)?. This allows invalid durations like P1YT to pass validation, as T can be matched with nothing following it. According to the ISO 8601 standard, if there are no time components, the T designator must be absent.
Adding a positive lookahead (?=\d) immediately after the T ensures it is only matched when followed by a valid time component (which must start with a digit).
💚 Proposed fix
/**
* The ISO 8601 / RFC 3339 duration regex. Matches strings like `P1D`,
* `PT5H30M`, `P1Y2M3DT4H5M6S`, with optional decimal values. A bare `P` or `PT`
* (no components) is rejected. See issue `#1497`.
*/
const ISO_DURATION_REGEX: RegExp =
- /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
+ /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ISO_DURATION_REGEX: RegExp = | |
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; | |
| const ISO_DURATION_REGEX: RegExp = | |
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@library/src/actions/isoDuration/isoDuration.ts` around lines 13 - 14, Update
ISO_DURATION_REGEX so the optional time group’s T designator requires a
following digit via a positive lookahead, preventing values such as P1YT from
matching while preserving valid hour, minute, and second components.
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="library/src/actions/isoDuration/isoDuration.ts">
<violation number="1" location="library/src/actions/isoDuration/isoDuration.ts:14">
P1: Regex accepts `P1DT` / `P1Y1M1DT` (T separator with no time components). Per ISO 8601, the T designator must be followed by at least one time component; a bare `T` should be rejected.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; | ||
|
|
There was a problem hiding this comment.
P1: Regex accepts P1DT / P1Y1M1DT (T separator with no time components). Per ISO 8601, the T designator must be followed by at least one time component; a bare T should be rejected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At library/src/actions/isoDuration/isoDuration.ts, line 14:
<comment>Regex accepts `P1DT` / `P1Y1M1DT` (T separator with no time components). Per ISO 8601, the T designator must be followed by at least one time component; a bare `T` should be rejected.</comment>
<file context>
@@ -0,0 +1,88 @@
+ * (no components) is rejected. See issue #1497.
+ */
+const ISO_DURATION_REGEX: RegExp =
+ /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
+
+/**
</file context>
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; | |
| /^P(?!T?$)(?:(\d+(?:\.\d+)?)Y)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)W)?(?:(\d+(?:\.\d+)?)D)?(?:T(?=\d)(?:(\d+(?:\.\d+)?)H)?(?:(\d+(?:\.\d+)?)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; |
Closes #1497.
Add an
isoDuration(message?)action that validates a string is an ISO 8601 / RFC 3339 duration (P[n]Y[n]M[n]W[n]DT[n]H[n]M[n]S, e.g.P1D,PT5H30M,P1Y2M3DT4H5M6S). Decimal values are supported. A barePorPT(no components) is rejected via a negative lookahead.Summary by CodeRabbit