fix(validation): update email validation to accept TLDs up to 6 characters #34112
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe update addresses a bug in the Changes
Sequence Diagram(s)Not applicable for these changes, as they primarily involve regex update and test case addition without significant control flow modifications. Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/widgets/InputWidgetV2/widget/derived.js (1 hunks)
- app/client/src/widgets/InputWidgetV2/widget/derived.test.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/InputWidgetV2/widget/derived.test.ts
Additional context used
Biome
app/client/src/widgets/InputWidgetV2/widget/derived.js
[error] 74-76: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
Additional comments not posted (1)
app/client/src/widgets/InputWidgetV2/widget/derived.js (1)
75-75: Updated email validation regex to allow TLDs up to 6 characters.
| * */ | ||
| const emailRegex = new RegExp( | ||
| /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,4})+$/, | ||
| /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,6})+$/, |
There was a problem hiding this comment.
Wrap the emailRegex declaration in a block to restrict its scope within the switch case.
- const emailRegex = new RegExp(/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,6})+$/);
+ {
const emailRegex = new RegExp(/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,6})+$/);
if (!emailRegex.test(value)) {
return false;
} else if (parsedRegex) {
return parsedRegex.test(props.inputText);
} else {
return true;
}
}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.
| /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,6})+$/, | |
| { | |
| const emailRegex = new RegExp(/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,6})+$/); | |
| if (!emailRegex.test(value)) { | |
| return false; | |
| } else if (parsedRegex) { | |
| return parsedRegex.test(props.inputText); | |
| } else { | |
| return true; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/widgets/InputWidgetV2/widget/derived.test.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/src/widgets/InputWidgetV2/widget/derived.test.ts
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
|
Reopening this. We will review and respond back. |
There was a problem hiding this comment.
Hey @razelos
I went through the RFC 5322 specification:
Max allowed chars in TLD: 64.
Real-world max seen: XN--VERMGENSBERATUNG-PWB - 24 chars.
Additionally, this article from Wix gives examples of up to 10 chars.
If we just make it 6 then we are just reacting to the requirement. IMO we should make a long term solution with going ahead with 10 chars. Thoughts?
2 participants
Fix input field for Email not validating long TLD
After reviewing issue #33837 I changed the default email validation logic to accept TLDs up to 6 letters long. Also added a test to test the [email protected] email.
Fixes #33837
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
Tests