Skip to content
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

fix: use a loose and future-oriented way to verify Chinese mobile phone numbers #1682

Merged
merged 1 commit into from
Oct 30, 2021

Conversation

yisibl
Copy link
Contributor

@yisibl yisibl commented Jun 9, 2021

The current zh-CN mobile phone number verification rules are too strict. With the increasing number of mobile phone numbers in China, this regularity will become difficult to maintain.

To this end, we should follow the official document (《电信网编号计划(2017年版)》) only need to verify the first two digits. This is a word document. For the convenience of viewing, I made a screenshot.

In this table, all mobile phone numbers from 13 to 19 exist except those starting with 12, and the addition of mobile phone numbers starting with 92 and 98 will be considered in the future. For better compatibility with the future, this PR also added 92/98 support. In addition, this PR does not consider IoT numbers.

image

ps: This library maintains a more detailed operator mobile phone number: https://github.com/VincentSit/ChinaMobilePhoneNumberRegex#rules

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@@ -114,7 +114,7 @@ const phones = {
'uk-UA': /^(\+?38|8)?0\d{9}$/,
'uz-UZ': /^(\+?998)?(6[125-79]|7[1-69]|88|9\d)\d{7}$/,
'vi-VN': /^(\+?84|0)((3([2-9]))|(5([2689]))|(7([0|6-9]))|(8([1-9]))|(9([0-9])))([0-9]{7})$/,
'zh-CN': /^((\+|00)86)?1([3456789][0-9]|4[579]|6[67]|7[01235678]|9[012356789])[0-9]{8}$/,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already 1([3456789][0-9], the following |4[579]|6[67]|7[01235678]|9[012356789] is meaningless.

@yisibl
Copy link
Contributor Author

yisibl commented Jun 9, 2021

cc @mlinquan Because of the introduction of complex verification rules in this PR. #801

@mlinquan
Copy link
Contributor

mlinquan commented Jun 9, 2021

@yisibl 我那个PR是3年前提交的,确实有点过时了。

@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #1682 (efc738e) into master (e08e79a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1682   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          101       101           
  Lines         1862      1862           
=========================================
  Hits          1862      1862           
Impacted Files Coverage Δ
src/lib/isMobilePhone.js 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e08e79a...efc738e. Read the comment docs.

mlinquan
mlinquan previously approved these changes Jun 23, 2021
Copy link
Contributor

@mlinquan mlinquan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adopt

@profnandaa profnandaa added needs-more-review 🧹 needs-update For PRs that need to be updated before landing 🎉 first-pr labels Jul 16, 2021
@profnandaa
Copy link
Member

Thanks for your PR, please fix the merge conflicts.

@yisibl
Copy link
Contributor Author

yisibl commented Jul 16, 2021

@profnandaa Done.

Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@profnandaa
Copy link
Member

/cc. @ezkemboi @tux-tn

@yisibl
Copy link
Contributor Author

yisibl commented Aug 4, 2021

@ezkemboi @tux-tn PTAL

@yisibl
Copy link
Contributor Author

yisibl commented Oct 24, 2021

Can we merge now?

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yisibl Sorry for the long delay! Your PR will be merged asap

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed and removed 🧹 needs-update For PRs that need to be updated before landing needs-more-review labels Oct 26, 2021
@yisibl
Copy link
Contributor Author

yisibl commented Oct 29, 2021

@tux-tn Thanks!

@profnandaa profnandaa merged commit 6b213cf into validatorjs:master Oct 30, 2021
theteladras pushed a commit to theteladras/validator.js that referenced this pull request Oct 30, 2021
profnandaa pushed a commit that referenced this pull request Oct 31, 2021
profnandaa pushed a commit that referenced this pull request Oct 31, 2021
@yisibl yisibl deleted the fix-zh-cn-phone branch November 1, 2021 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 first-pr ready-to-land For PRs that are reviewed and ready to be landed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants