feat: set pattern constraint for regular expressions#1188
Open
transparent-citizen wants to merge 3 commits intoedmundhung:mainfrom
Open
feat: set pattern constraint for regular expressions#1188transparent-citizen wants to merge 3 commits intoedmundhung:mainfrom
pattern constraint for regular expressions#1188transparent-citizen wants to merge 3 commits intoedmundhung:mainfrom
Conversation
🦋 Changeset detectedLatest commit: b59535e The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@conform-to/dom
@conform-to/react
@conform-to/valibot
@conform-to/validitystate
@conform-to/yup
@conform-to/zod
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a new exported utility Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
transparent-citizen
force-pushed
the
main
branch
from
7dcb74c to
18ff2e4
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-dom/util.ts`:
- Around line 129-134: The single-pattern branch returns first.source which
changes semantics to full-string match; instead make the single-pattern case use
the same lookahead construction as the multi-pattern branch so unanchored
regexes remain substring-style (use patterns and first variables and return the
`(?=.*...)` based string instead of first.source) — update the logic in util.ts
around the patterns/first handling so both single and multiple patterns generate
`^` + lookaheads + `.*$`, ensuring downstream generators in
conform-valibot/constraint.ts and conform-zod/* preserve original regex
semantics.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9d4cafac-e70c-43f0-8c68-fd14f058eea7
📒 Files selected for processing (8)
packages/conform-dom/future/index.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/conform-dom/util.ts (1)
129-134:⚠️ Potential issue | 🟠 MajorSingle-regex branch changes regex semantics and can over-reject valid inputs.
This issue was flagged in a previous review. The single-pattern case returns
first.sourcedirectly, which makes unanchored regexes behave as full-string matches in HTML (thepatternattribute has implicit^...$anchors). However, Zod/Valibot's.regex()usesRegExp.test()which matches substrings.Example: A string
"abcD"passes.regex(/[A-Z]/)in Zod, butpattern="[A-Z]"fails in browser validation because HTML requires the entire value to match.The single-pattern case should use the same lookahead strategy as multi-pattern for consistent semantics:
💡 Proposed fix
export function combinePatterns(patterns: RegExp[]): string { if (patterns.length === 0) { return ''; } - const first = patterns[0]; - if (first && patterns.length === 1) { - return first.source; - } - - return `^${patterns.map((p) => `(?=.*${p.source})`).join('')}.*$`; + return `^${patterns.map((p) => `(?=.*(?:${p.source}))`).join('')}.*$`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-dom/util.ts` around lines 129 - 134, The single-pattern branch currently returns first.source directly which changes semantics; instead always build the anchored lookahead pattern used for multi-patterns so HTML pattern semantics match RegExp.test() behavior. Replace the special-case return of first.source (the code referencing first and patterns) with the same construction used below: return an anchored string using patterns.map(p => `(?=.*${p.source})`).join('') wrapped with ^ and .*$ so both single and multiple patterns use the lookahead strategy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/pattern-constraint-regex.md:
- Around line 10-22: The example is missing the zod import used by schema
(symbols: z and schema); add the zod import at the top of the snippet so
z.object and z.string() resolve (e.g., import the named export that provides z
from the 'zod' package) and ensure the import sits alongside the existing
imports (configureForms, getConstraints, InputHTMLAttributes).
---
Duplicate comments:
In `@packages/conform-dom/util.ts`:
- Around line 129-134: The single-pattern branch currently returns first.source
directly which changes semantics; instead always build the anchored lookahead
pattern used for multi-patterns so HTML pattern semantics match RegExp.test()
behavior. Replace the special-case return of first.source (the code referencing
first and patterns) with the same construction used below: return an anchored
string using patterns.map(p => `(?=.*${p.source})`).join('') wrapped with ^ and
.*$ so both single and multiple patterns use the lookahead strategy.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0f283ca0-ddcf-401a-9ba7-4d55b0359757
📒 Files selected for processing (9)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
from
18ff2e4 to
2b0c670
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/pattern-constraint-regex.md:
- Around line 30-31: Update the inline comment that shows the expected combined
pattern to match the actual output from combinePatterns: change the example from
'^(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#$%^&*]).*$' to include the non-capturing groups
produced by combinePatterns, i.e.
'^(?=.*(?:[A-Z]))(?=.*(?:[0-9]))(?=.*(?:[!@#$%^&*])).*$'; ensure the comment
near where metadata.pattern is assigned reflects this exact format and
references combinePatterns behavior.
In `@packages/conform-dom/util.ts`:
- Around line 117-130: The JSDoc example for combinePatterns is inaccurate: the
function takes a RegExp[] (not string[]) and the implementation wraps each
pattern in a non-capturing group, so update the documentation for the
combinePatterns function to show RegExp[] input and the actual output (e.g.,
['[A-Z]', '[0-9]'] -> '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$'); ensure the example
uses RegExp-style patterns (or refactor the function if you prefer string
inputs) and adjust the comment text to match the implemented return value
exactly.
In `@packages/conform-zod/v4/tests/constraint.test.ts`:
- Around line 461-484: The combinePatterns implementation is wrapping each regex
source in an unnecessary non-capturing group which yields incorrect lookahead
patterns; update the combinePatterns function in util.ts to build lookaheads
using the raw pattern source (use `(?=.*${p.source})` for each RegExp p) instead
of `(?=.*(?:${p.source}))`, so the returned string becomes
`^${patterns.map(...).join('')}.*$` with direct p.source insertion and no extra
`(?:...)` wrapping.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1f7618c3-8d72-4b08-9593-c1106bd84f8c
📒 Files selected for processing (9)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
2 times, most recently
from
f028ada to
512dc23
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/conform-dom/util.ts (1)
117-123: 🧹 Nitpick | 🔵 TrivialJSDoc has minor inconsistencies with the implementation.
Line 119 states "For a single pattern, returns the pattern source as-is" but the implementation always wraps patterns in the lookahead format regardless of count.
Line 122's example shows
['[A-Z]', '[0-9]'](string array notation) but the function signature takesRegExp[].📝 Suggested documentation fix
/** * Combines multiple regex patterns into a single HTML pattern attribute string. - * For a single pattern, returns the pattern source as-is. * For multiple patterns, combines them using lookahead assertions that must ALL match. * - * Example: ['[A-Z]', '[0-9]'] -> '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$' + * Example: [/[A-Z]/, /[0-9]/] -> '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$' */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-dom/util.ts` around lines 117 - 123, The JSDoc is out of sync with the implementation: update the comment for the function that combines multiple RegExp patterns into an HTML pattern attribute (the function that accepts RegExp[]) to reflect that it always wraps patterns in lookahead assertions (it does not return a single pattern source as-is), and change the example to show RegExp values (e.g. [/ [A-Z]/, /[0-9]/] or the literal /[A-Z]/, /[0-9]/) instead of string array notation so the signature RegExp[] matches the example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-valibot/tests/constraint.test.ts`:
- Around line 390-402: The assertion in the test for getValibotConstraint
currently requires exact pattern string equality which makes the test brittle;
update the test in constraint.test.ts to assert the structural parts (e.g.,
empty.required, singlePattern.required, multiplePatterns.required) remain true
but replace exact pattern string equality with semantic checks: extract the
pattern value returned by getValibotConstraint(schema), construct RegExp objects
from those pattern strings, and assert they accept known good inputs and reject
known bad inputs for singlePattern and multiplePatterns (e.g., uppercase-only vs
lowercase, and combinations with digits) so logically equivalent regex rewrites
won't break the test.
---
Duplicate comments:
In `@packages/conform-dom/util.ts`:
- Around line 117-123: The JSDoc is out of sync with the implementation: update
the comment for the function that combines multiple RegExp patterns into an HTML
pattern attribute (the function that accepts RegExp[]) to reflect that it always
wraps patterns in lookahead assertions (it does not return a single pattern
source as-is), and change the example to show RegExp values (e.g. [/ [A-Z]/,
/[0-9]/] or the literal /[A-Z]/, /[0-9]/) instead of string array notation so
the signature RegExp[] matches the example.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8a7566df-2a95-4f89-b768-e078763d95c0
📒 Files selected for processing (9)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/conform-valibot/tests/constraint.test.ts (1)
390-402: 🧹 Nitpick | 🔵 TrivialUse semantic regex assertions to avoid brittle serialization coupling.
Line 390-Line 402 currently lock the test to one exact pattern string form; logically equivalent rewrites of
combinePatternswould cause noisy failures.♻️ Suggested test hardening
- expect(getValibotConstraint(schema)).toEqual({ + const constraint = getValibotConstraint(schema); + expect(constraint).toEqual({ empty: { required: true, }, singlePattern: { required: true, pattern: '^(?=.*(?:^[A-Z]+$)).*$', }, multiplePatterns: { required: true, pattern: '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$', }, }); + + const singlePattern = new RegExp(constraint.singlePattern.pattern!); + expect(singlePattern.test('ABC')).toBe(true); + expect(singlePattern.test('AbC')).toBe(false); + + const multiplePatterns = new RegExp(constraint.multiplePatterns.pattern!); + expect(multiplePatterns.test('A1')).toBe(true); + expect(multiplePatterns.test('ABC')).toBe(false); + expect(multiplePatterns.test('123')).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-valibot/tests/constraint.test.ts` around lines 390 - 402, The test currently asserts exact serialized pattern strings from getValibotConstraint which is brittle; instead assert semantics: keep checking that getValibotConstraint(schema).empty.required and .singlePattern.required/.multiplePatterns.required are true, and for the pattern fields verify their semantics by (a) treating the returned pattern string as a RegExp (e.g., new RegExp(pattern)) and asserting it matches positive example strings and rejects negative examples, or (b) assert the pattern string contains the expected lookahead fragments like '(?=.*[A-Z])' and '(?=.*[0-9])' using string matching—refer to getValibotConstraint and the schema/singlePattern/multiplePatterns keys to locate where to change the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-dom/util.ts`:
- Around line 124-129: The combinePatterns function currently drops RegExp flags
by using p.source which can produce incorrect HTML pattern attributes for
flagged regexes; modify combinePatterns (and its return type) to detect any
flagged RegExp (e.g., check p.flags or p.constructor.name) and return undefined
immediately when any pattern has flags instead of producing a string, otherwise
keep the existing combined `^...$` behavior using p.source; update any callers
expecting a string to handle the new undefined case so flagged regexes are not
emitted to the HTML pattern attribute.
---
Duplicate comments:
In `@packages/conform-valibot/tests/constraint.test.ts`:
- Around line 390-402: The test currently asserts exact serialized pattern
strings from getValibotConstraint which is brittle; instead assert semantics:
keep checking that getValibotConstraint(schema).empty.required and
.singlePattern.required/.multiplePatterns.required are true, and for the pattern
fields verify their semantics by (a) treating the returned pattern string as a
RegExp (e.g., new RegExp(pattern)) and asserting it matches positive example
strings and rejects negative examples, or (b) assert the pattern string contains
the expected lookahead fragments like '(?=.*[A-Z])' and '(?=.*[0-9])' using
string matching—refer to getValibotConstraint and the
schema/singlePattern/multiplePatterns keys to locate where to change the
assertions.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d478dc02-fd45-4074-9bdb-0efa0dc747b8
📒 Files selected for processing (9)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
4 times, most recently
from
9103975 to
2e0da81
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/pattern-constraint-regex.md:
- Line 8: The markdown file starts prose immediately after frontmatter and
triggers markdownlint MD041; add a top-level H1 heading (e.g., "# Pattern
constraint regex") at the top of the document immediately after the existing
frontmatter so the file has an H1 before any prose; ensure the heading text
describes the change (references to "pattern-constraint-regex" or similar) and
leave the rest of the content unchanged.
In `@packages/conform-dom/tests/util.test.ts`:
- Around line 39-42: Tests only cover 'g' flag rejection; add cases to ensure
allowed flags ('u' and 'v') and other unsupported flags are enforced by
combinePatterns. In util.test.ts add an assertion that combinePatterns([/a/u,
/b/u]) (or similar) returns a combined RegExp (not undefined) and an assertion
that combinePatterns([/A/i]) (or mix with other flags) returns undefined;
reference combinePatterns and the whitelist logic in util.ts (flags 'u' and 'v')
to guide expectations.
In `@packages/conform-dom/util.ts`:
- Around line 127-137: The exported function combinePatterns should declare an
explicit return type of string | undefined to make its public contract clear;
update the function signature for combinePatterns (the exported function in
util.ts) to return string | undefined instead of relying on inference, leaving
the implementation unchanged so callers see the precise return type.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 02279444-904a-45f4-91eb-eaed7d1cbae4
📒 Files selected for processing (10)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/tests/util.test.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
2 times, most recently
from
ce76ea1 to
fe25719
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/conform-dom/tests/util.test.ts (1)
39-42: 🧹 Nitpick | 🔵 TrivialConsider adding test cases for allowed flags (
u/v).The tests cover unsupported flags (
g,i) returningundefined, but don't verify that patterns withuorvflags (which are stripped) still produce valid output.📝 Suggested test additions
// Patterns with unsupported flags expect(combinePatterns([/[A-Z]/g])).toBeUndefined(); expect(combinePatterns([/[A-Z]/, /[0-9]/i])).toBeUndefined(); + + // Patterns with allowed flags (u/v are stripped, should still work) + expect(combinePatterns([/[A-Z]/u])).toBe('^(?=.*(?:[A-Z])).*$'); + expect(combinePatterns([/[A-Z]/v, /[0-9]/u])).toBe( + '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$', + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-dom/tests/util.test.ts` around lines 39 - 42, The test suite for combinePatterns currently checks that unsupported flags (g, i) yield undefined but lacks coverage for allowed flags; add tests invoking combinePatterns with patterns using the u and v flags (e.g. /.../u and /.../v) and assert the result is not undefined and correctly combines the pattern bodies (i.e., flags are stripped and the resulting RegExp or pattern string reflects the combined content); update util.test.ts near the other combinePatterns tests and reference the combinePatterns function in your assertions..changeset/pattern-constraint-regex.md (1)
8-8:⚠️ Potential issue | 🟡 MinorAdd an H1 after frontmatter to satisfy markdownlint MD041.
The first content line is prose; this should start with a top-level heading.
Suggested fix
+# Map regex constraints to `pattern` + Mapped regex constraints to `pattern`, enabling advanced client-side validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/pattern-constraint-regex.md at line 8, Add a top-level heading immediately after the YAML frontmatter in .changeset/pattern-constraint-regex.md so the first content line is not plain prose; insert an H1 (for example referencing the change like "# Pattern constraint regex" or a generic "# Changelog") directly after the closing frontmatter delimiter (---) and before the existing line "Mapped regex constraints to `pattern`, enabling advanced client-side validation." to satisfy markdownlint MD041.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.changeset/pattern-constraint-regex.md:
- Line 8: Add a top-level heading immediately after the YAML frontmatter in
.changeset/pattern-constraint-regex.md so the first content line is not plain
prose; insert an H1 (for example referencing the change like "# Pattern
constraint regex" or a generic "# Changelog") directly after the closing
frontmatter delimiter (---) and before the existing line "Mapped regex
constraints to `pattern`, enabling advanced client-side validation." to satisfy
markdownlint MD041.
In `@packages/conform-dom/tests/util.test.ts`:
- Around line 39-42: The test suite for combinePatterns currently checks that
unsupported flags (g, i) yield undefined but lacks coverage for allowed flags;
add tests invoking combinePatterns with patterns using the u and v flags (e.g.
/.../u and /.../v) and assert the result is not undefined and correctly combines
the pattern bodies (i.e., flags are stripped and the resulting RegExp or pattern
string reflects the combined content); update util.test.ts near the other
combinePatterns tests and reference the combinePatterns function in your
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fefe9485-13b4-4cf5-bc04-6fa8d26e9633
📒 Files selected for processing (10)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/tests/util.test.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
from
fe25719 to
decc258
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-dom/tests/util.test.ts`:
- Around line 39-42: Add tests asserting that combinePatterns accepts allowed
flags 'u' and 'v': call combinePatterns with arrays containing patterns that
include the 'u' flag and the 'v' flag (e.g., /.../u and /.../v) and assert the
result is not undefined and is a RegExp (or matches expected combined behavior).
Update packages/conform-dom/tests/util.test.ts near the existing flag tests to
include these assertions referencing combinePatterns so we confirm allowed flags
produce a valid combined pattern.
In `@packages/conform-zod/v4/tests/constraint.test.ts`:
- Around line 461-484: The test "regex patterns" currently only asserts the
exact pattern strings returned by getZodConstraint but should also include
semantic runtime checks to ensure those pattern strings actually match the
intended inputs; update the test that builds the schema (variable schema with
fields empty, singlePattern, multiplePatterns) to, after calling
getZodConstraint, construct RegExp instances from the returned pattern values
for singlePattern and multiplePatterns and assert RegExp.test(...) against
representative matching and non-matching sample strings (e.g., uppercase-only
and numeric-containing samples) so the patterns are validated semantically in
addition to string equality.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 60decdd8-3ca8-4b8a-9868-6d7050e23f96
📒 Files selected for processing (10)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/tests/util.test.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
Comment on lines
+39
to
+42
| // Patterns with unsupported flags | ||
| expect(combinePatterns([/[A-Z]/g])).toBeUndefined(); | ||
| expect(combinePatterns([/[A-Z]/, /[0-9]/i])).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add test coverage for allowed u and v flags.
The tests verify that unsupported flags (g, i) cause undefined to be returned, but there's no assertion confirming that patterns with the allowed u or v flags still produce a valid combined pattern.
♻️ Proposed test additions
// Patterns with unsupported flags
expect(combinePatterns([/[A-Z]/g])).toBeUndefined();
expect(combinePatterns([/[A-Z]/, /[0-9]/i])).toBeUndefined();
+
+ // Patterns with allowed flags (u and v)
+ expect(combinePatterns([/[A-Z]/u])).toBe('^(?=.*(?:[A-Z])).*$');
+ expect(combinePatterns([/[A-Z]/v, /[0-9]/])).toBe(
+ '^(?=.*(?:[A-Z]))(?=.*(?:[0-9])).*$',
+ );
});📝 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.
Suggested change
| // Patterns with unsupported flags | |
| expect(combinePatterns([/[A-Z]/g])).toBeUndefined(); | |
| expect(combinePatterns([/[A-Z]/, /[0-9]/i])).toBeUndefined(); | |
| }); | |
| // Patterns with unsupported flags | |
| expect(combinePatterns([/[A-Z]/g])).toBeUndefined(); | |
| expect(combinePatterns([/[A-Z]/, /[0-9]/i])).toBeUndefined(); | |
| // Patterns with allowed flags (u and v) | |
| expect(combinePatterns([/[A-Z]/u])).toBe('^(?=.*(?:[A-Z])).*$'); | |
| expect(combinePatterns([/[A-Z]/v, /[0-9]/])).toBe( | |
| '^(?=.*(?:[0-9]))(?=.*(?:[A-Z])).*$', | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-dom/tests/util.test.ts` around lines 39 - 42, Add tests
asserting that combinePatterns accepts allowed flags 'u' and 'v': call
combinePatterns with arrays containing patterns that include the 'u' flag and
the 'v' flag (e.g., /.../u and /.../v) and assert the result is not undefined
and is a RegExp (or matches expected combined behavior). Update
packages/conform-dom/tests/util.test.ts near the existing flag tests to include
these assertions referencing combinePatterns so we confirm allowed flags produce
a valid combined pattern.
Contributor
Author
There was a problem hiding this comment.
I added a test for the u flag, but chose to pass on the v flag. The syntax is relatively new, and requiring newer syntax might limit environment support.
@edmundhung: Willing to change this if you prefer.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/conform-dom/tests/util.test.ts (1)
39-45:⚠️ Potential issue | 🟡 MinorAdd explicit
v-flag coverage incombinePatternstests.Line [43] only verifies
uas supported. Add avcase too, so the supported-flag contract is fully locked and regressions are caught.♻️ Suggested test update
test('combinePatterns', () => { @@ // Supported flags expect(combinePatterns([/[A-Z]/u])).toBe('^(?=.*(?:[A-Z])).*$'); + + // `v` may not be available in all JS runtimes; gate to avoid parser/runtime issues + let supportsV = false; + try { + new RegExp('', 'v'); + supportsV = true; + } catch { + supportsV = false; + } + + if (supportsV) { + expect(combinePatterns([new RegExp('[A-Z]', 'v')])).toBe( + '^(?=.*(?:[A-Z])).*$', + ); + } });#!/bin/bash # Read-only verification: confirm util whitelist behavior and current test coverage. rg -n -C3 "combinePatterns|flags|u|v" packages/conform-dom/util.ts rg -n -C3 "Supported flags|Unsupported flags|combinePatterns\\(" packages/conform-dom/tests/util.test.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-dom/tests/util.test.ts` around lines 39 - 45, The test currently asserts that combinePatterns supports the 'u' flag but omits verifying support for the 'v' flag; update the tests around combinePatterns in util.test.ts to add an explicit case asserting that combinePatterns([/[A-Z]/v]) returns the expected combined string (matching the same format as the 'u' case), and also add a mixed supported-case if desired (e.g., a pattern with 'v' combined alone or with other supported flags) so the supported-flag contract for combinePatterns is locked and will fail on regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-dom/util.ts`:
- Around line 122-123: Update the JSDoc to match the implementation: state that
the function strips the `u` and `v` flags (because HTML pattern attributes
implicitly run with `v`) and only returns undefined when any other flags are
present; mention that `u`/`v` are silently removed rather than treated as an
error. Apply this change to the JSDoc for the pattern-handling function in
packages/conform-dom/util.ts so the comment aligns with the code that strips
`u`/`v` and rejects other flags.
---
Duplicate comments:
In `@packages/conform-dom/tests/util.test.ts`:
- Around line 39-45: The test currently asserts that combinePatterns supports
the 'u' flag but omits verifying support for the 'v' flag; update the tests
around combinePatterns in util.test.ts to add an explicit case asserting that
combinePatterns([/[A-Z]/v]) returns the expected combined string (matching the
same format as the 'u' case), and also add a mixed supported-case if desired
(e.g., a pattern with 'v' combined alone or with other supported flags) so the
supported-flag contract for combinePatterns is locked and will fail on
regressions.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4eaa0336-70e5-42c5-8e41-abbd4fcf6afa
📒 Files selected for processing (10)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/tests/util.test.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
2 times, most recently
from
5ebb455 to
4898385
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/conform-dom/util.ts (1)
122-123:⚠️ Potential issue | 🟡 MinorJSDoc still overstates flag rejection behavior.
The wording says “any pattern has flags,” but the implementation allows
u/vand only rejects other flags. Please align wording with code behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-dom/util.ts` around lines 122 - 123, Update the JSDoc in packages/conform-dom/util.ts that describes HTML pattern flag handling to match the code: state that the function only rejects patterns with disallowed flags (i.e., flags other than `u` and `v`), and that it strips `u` and `v` because HTML pattern attributes don't support flags; remove the phrase “any pattern has flags” and explicitly mention that `u`/`v` are allowed/stripped while other flags cause undefined to be returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/conform-dom/util.ts`:
- Around line 122-123: Update the JSDoc in packages/conform-dom/util.ts that
describes HTML pattern flag handling to match the code: state that the function
only rejects patterns with disallowed flags (i.e., flags other than `u` and
`v`), and that it strips `u` and `v` because HTML pattern attributes don't
support flags; remove the phrase “any pattern has flags” and explicitly mention
that `u`/`v` are allowed/stripped while other flags cause undefined to be
returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96c51260-d6ac-4a7a-adf8-0476a0bc53e7
📒 Files selected for processing (10)
.changeset/pattern-constraint-regex.mdpackages/conform-dom/future/index.tspackages/conform-dom/tests/util.test.tspackages/conform-dom/util.tspackages/conform-valibot/constraint.tspackages/conform-valibot/tests/constraint.test.tspackages/conform-zod/default/constraint.tspackages/conform-zod/default/tests/constraint.test.tspackages/conform-zod/v4/constraint.tspackages/conform-zod/v4/tests/constraint.test.ts
transparent-citizen
force-pushed
the
main
branch
from
4898385 to
8517a03
Compare
Contributor
Author
|
@edmundhung Thanks for setting up CodeRabbit! A lot of its suggestions were quite helpful. Looking forward to hearing from you! 🙂 |
transparent-citizen
force-pushed
the
main
branch
2 times, most recently
from
162d634 to
c6760a0
Compare
transparent-citizen
force-pushed
the
main
branch
3 times, most recently
from
a4bd405 to
64b2d10
Compare
transparent-citizen
force-pushed
the
main
branch
from
64b2d10 to
afb5964
Compare
edmundhung
reviewed
Apr 18, 2026
Owner
edmundhung
left a comment
edmundhung
left a comment
There was a problem hiding this comment.
Love it! Thanks for working on this 👍🏼
Comment on lines
+132
to
+134
| if (patterns.some((p) => p.flags.replace(/[uv]/g, ''))) { | ||
| return undefined; | ||
| } |
Owner
There was a problem hiding this comment.
I would suggest making it best-effort, so remaining valid regex should still be used to generate a HTML pattern string.
I also wonder if we should validate the regex manually using the v flag, like this:
try {
new RegExp(pattern, 'v');
return true;
} catch {
return false;
}
What do you think?
Contributor
Author
There was a problem hiding this comment.
I looked into the unsupported flags a bit more. From what I can tell, we can safely drop all of them without reducing the validity of the pattern, the only exception being i.
However, /a/i and /[aA]/ describe the same grammar, and similarly we can map /[a-z]/i to /[a-zA-Z]/ to make the regex flagless. It looks feasible to map all i-flagged regexes to flagless regexes.
I think validity checks like the one you showed are reasonable to do, but I think tests are a more suitable environment for this code. Runtime verification makes Conform pay a runtime cost, as it would load a new RegExp object into memory and compile the pattern to a finite-state machine, when all it needed to do was to combine the input patterns.
Is it at all possible to enter the catch case? Given that the inputs to combinePatterns are RegExp objects with valid patterns, and we're placing them in isolated slots using (?:...), it couldn't possibly yield an unparseable regex, can it?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR lets Conform construct an HTML
patternattribute based on regular expressions from zod v3, v4, and valibot schemas.It's worth noting that these validators allow multiple regexes for string schemas. For example in zod,
matches strings containing at least one "a" and one "b" in any order. But because
<input>accepts only onepatternattribute, multiple regexes need to be combined into one, using positive lookahead assertions:The correctness of
outis supported by this jsfiddle featuring an input whose text turns green if and only if it matches bothinpatterns.Regular expressions in the HTML
patternattribute run with thevflag by default (used to beu). These supported flags are stripped, while unsupported flags will not be mapped to a pattern.Generated Summary