-
-
Couldn't load subscription status.
- Fork 281
Feat: add domain() validation action (ASCII domains) #1284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new domain() validation action for validating ASCII domain names in the Valibot library. The implementation focuses on providing a conservative regex-based approach to domain validation.
- Adds core
domain()action with TypeScript interfaces and comprehensive test coverage - Implements ASCII-only domain validation with support for Punycode IDN labels
- Creates complete API documentation including examples and related actions
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
library/src/regex.ts |
Adds DOMAIN_REGEX constant for ASCII domain validation |
library/src/actions/domain/domain.ts |
Core implementation of domain validation action with interfaces |
library/src/actions/domain/domain.test.ts |
Comprehensive test suite covering valid/invalid domain cases |
library/src/actions/domain/domain.test-d.ts |
TypeScript type checking tests for the domain action |
library/src/actions/index.ts |
Exports the new domain action module |
website/src/routes/api/menu.md |
Updates API menu to include domain action and types |
website/src/routes/api/(actions)/domain/index.mdx |
Complete API documentation with examples and usage guidelines |
website/src/routes/api/(actions)/url/index.mdx |
Cross-references the new domain action from URL documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
commit: |
|
Thank you for this PR! Do "normal" domains only support a single |
The difficulty is that the specification does not have regular expressions. And in the literature, for example, https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch08s15.html, fairly primitive regular expressions are presented. Determining the correct domain is more about procedures, not about patterns. We will not be able to achieve perfect validation, but we can somehow get closer to it. and just for information, the source of truth for all TLDs: |
|
Would your recommendation be to introduce |
I think we can go ahead with only ASCII checking, which we call "regular" or "normal" domain. Then we can add But before that, I think we should make the current zod regular expression a bit stricter:
Seems pretty safe. The final result: /^(?=.{1,253}$)([a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z]{2,63}$/i;After that, I will add new tests and the documentation description. UPD: Done |
…emove IDN/Punycode references
…ode before validation
982e51c to
a17cc7d
Compare
Add domain() validation action (ASCII domains) + docs
Closes #1277
Summary
domain()validation action for ASCII domain namesRationale
UPD: Zod regex with small updates #1284 (comment)
Decisions
localhostand bare intranet-like hostnames are considered invalid fordomain()(by design)Limitations
Discuss
We can switch to a regex that supports Punycode TLD (and total length) if desired
The advantage of this solution is that we can add
toPunycode()and use it beforedomain()for unicode domains.If we go with a solution that only checks simple cases, we will face the need to add a separate
rfcDomainaction, as it happens now withemail+rfcEmail.What do you think about this?