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

feat(phone)!: add new style parameter #2578

Merged
merged 20 commits into from
Mar 12, 2024

Conversation

matthewmayer
Copy link
Contributor

@matthewmayer matthewmayer commented Dec 13, 2023

fix #1542

Script for doing the grunt work of renaming the formats.ts and generating the initial raw and national files

const {
    allLocales
} = require("@faker-js/faker")
const lpn = require("libphonenumber-js")
const fs = require("fs")
const fsx = require("fs-extra")
//guess the country for locales with no country attached
const fallbacks = {
    "az": "AZ",
    "da": "DK",
    "de": "DE",
    "dv": "MV",
    "el": "GR",
    "en": "US",
    "es": "ES",
    "fa": "IR",
    "fr": "FR",
    "he": "IL",
    "hr": "HR",
    "hu": "HU",
    "hy": "AM",
    "it": "IT",
    "ja": "JP",
    "ko": "KR",
    "lv": "LV",
    "mk": "MK",
    "ne": "NP",
    "nl": "NL",
    "pl": "PL",
    "ro": "RO",
    "ru": "RU",
    "sk": "SK",
    "sv": "SE",
    "th": "TH",
    "tr": "TR",
    "uk": "UA",
    "vi": "VN"
}

for (let locale of Object.keys(allLocales)) {
    //loop through all locales that have an existing formats.ts
    const filename = "../faker/src/locales/" + locale + "/phone_number/formats.ts"
    if (fs.existsSync(filename)) {
        //extract formats array from TS
        const data = fs.readFileSync(filename, "utf8")
        let arrayString = data.replace('export default ', '').replace(';', '');
        let formats = eval(arrayString)
        //figure out the country code
        let country = null
        if (locale.includes("_")) {
            country = locale.split("_")[1]
        } else if (fallbacks[locale]) {
            country = fallbacks[locale]
        }
        if (country) {
            let countryCode = lpn.getCountryCallingCode(country).toString()
            let nationals = []
            let raws = []
            for (let format of formats) {
                if (!format.includes("x")) { //skip extensions
                    //replace each # in the format string with placeholder
                    //we need to make sure its not one of the ones in the format or country code
                    //so we can losslessly restore it later
                    //so start at 2, and increment until we find a free one
                    let hash = 2
                    while (format.includes(hash) || countryCode.includes(hash)) {
                        hash++
                    }
                    //do the same for !
                    let pling = hash + 1
                    while (format.includes(pling) || countryCode.includes(pling)) {
                        pling++
                    }
                    //replace the # and ! with the placeholders
                    let phone = format.replaceAll('#', hash).replaceAll('!', pling)
                    //later we'll need to restore them
                    const restorePlaceholders = (string) => {
                        return string.replaceAll(hash, "#").replaceAll(pling, "!")
                    }
                    try {
                        //parse the phone number
                        const phoneNumber = lpn.parsePhoneNumber(phone, country)
                        //format national
                        let national = restorePlaceholders(phoneNumber.formatNational())
                        nationals.push(national)
                        //format raw
                        const raw = restorePlaceholders(phoneNumber.getURI().replace("tel:", ""))
                        raws.push(raw)
                    } catch (e) {
                        //
                    }
                }
                //move the file from formats.ts to human.ts
                try {
                    fsx.moveSync(filename, filename.replace("formats", "human"))
                } catch (e) {

                }
                //write out the new files, avoiding dupes
                fs.writeFileSync(filename.replace("formats", "national"), "export default " + JSON.stringify([...new Set(nationals)]) + ";")
                fs.writeFileSync(filename.replace("formats", "raw"), "export default " + JSON.stringify([...new Set(raws)]) + ";")

            }
        }
    }
}

@matthewmayer matthewmayer added do NOT merge yet Do not merge this PR into the target branch yet breaking change Cannot be merged when next version is not a major release m: phone Something is referring to the phone module labels Dec 13, 2023
@matthewmayer matthewmayer self-assigned this Dec 13, 2023
@matthewmayer matthewmayer added this to the v9.x milestone Dec 13, 2023
@ST-DDT ST-DDT modified the milestones: v9.x, v9.0 Feb 8, 2024
@ST-DDT ST-DDT changed the title feat(phone): add new style parameter feat(phone)!: add new style parameter Feb 8, 2024
@ST-DDT ST-DDT changed the title feat(phone)!: add new style parameter feat!(phone): add new style parameter Feb 8, 2024
@ST-DDT ST-DDT removed the do NOT merge yet Do not merge this PR into the target branch yet label Feb 8, 2024
@ST-DDT ST-DDT changed the title feat!(phone): add new style parameter feat(phone)!: add new style parameter Feb 8, 2024
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

Attention: Patch coverage is 99.35897% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 99.57%. Comparing base (aade09b) to head (d6dbf8f).

❗ Current head d6dbf8f differs from pull request most recent head 7b8bcd6. Consider uploading reports for the commit 7b8bcd6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2578      +/-   ##
==========================================
- Coverage   99.57%   99.57%   -0.01%     
==========================================
  Files        2859     3034     +175     
  Lines      248602   250972    +2370     
  Branches      985      994       +9     
==========================================
+ Hits       247541   249900    +2359     
- Misses       1061     1072      +11     
Files Coverage Δ
src/locales/af_ZA/phone_number/format/human.ts 100.00% <ø> (ø)
src/locales/af_ZA/phone_number/format/index.ts 100.00% <100.00%> (ø)
...locales/af_ZA/phone_number/format/international.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/phone_number/format/national.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/phone_number/index.ts 100.00% <100.00%> (ø)
src/locales/ar/index.ts 100.00% <ø> (ø)
src/locales/az/phone_number/format/human.ts 100.00% <ø> (ø)
src/locales/az/phone_number/format/index.ts 100.00% <100.00%> (ø)
...rc/locales/az/phone_number/format/international.ts 100.00% <100.00%> (ø)
src/locales/az/phone_number/format/national.ts 100.00% <100.00%> (ø)
... and 287 more

... and 3 files with indirect coverage changes

@matthewmayer matthewmayer marked this pull request as ready for review February 9, 2024 02:05
@matthewmayer matthewmayer requested a review from a team February 9, 2024 02:05
@matthewmayer matthewmayer requested a review from a team as a code owner February 9, 2024 02:05
@matthewmayer
Copy link
Contributor Author

We'll need rebase after #2712 lands

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

I forgot this had a pending comment...

src/definitions/phone_number.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added c: feature Request for new feature needs rebase There is a merge conflict p: 1-normal Nothing urgent labels Mar 4, 2024
@ST-DDT ST-DDT removed the needs rebase There is a merge conflict label Mar 5, 2024
ST-DDT
ST-DDT previously approved these changes Mar 5, 2024
src/locales/ar/phone_number/formats.ts Outdated Show resolved Hide resolved
src/modules/phone/index.ts Outdated Show resolved Hide resolved
src/modules/phone/index.ts Outdated Show resolved Hide resolved
src/modules/phone/index.ts Outdated Show resolved Hide resolved
src/modules/phone/index.ts Outdated Show resolved Hide resolved
src/modules/phone/index.ts Show resolved Hide resolved
@ST-DDT ST-DDT dismissed their stale review March 5, 2024 17:31

Wrong button

src/modules/phone/index.ts Outdated Show resolved Hide resolved
matthewmayer and others added 2 commits March 6, 2024 15:07
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
Co-authored-by: Eric Cheng <ericcheng9316@gmail.com>
src/modules/phone/index.ts Show resolved Hide resolved
@ST-DDT
Copy link
Member

ST-DDT commented Mar 6, 2024

The description states it is still WIP. Is that outdated or what is left to do?

@matthewmayer
Copy link
Contributor Author

Nope sorry not WIP any more. Ready for final review.

@ST-DDT ST-DDT enabled auto-merge (squash) March 12, 2024 10:59
@ST-DDT ST-DDT merged commit e130549 into faker-js:next Mar 12, 2024
13 of 14 checks passed
@romain-sen
Copy link

I am sorry but I am lost, using parameters with faker.phone.number() with parameters is deprecated. So why this PR seems to add parameters ? Is the deprecation canceled or something ?

@ST-DDT
Copy link
Member

ST-DDT commented Apr 5, 2024

The new parameters are entirely different and incompatible with the previous ones.

@romain-sen
Copy link

In which version will it be available ?

@ST-DDT
Copy link
Member

ST-DDT commented Apr 5, 2024

In which version will it be available ?

v9.0.0 (+preview releases)

We plan to release an alpha soon. Maybe next week?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Cannot be merged when next version is not a major release c: feature Request for new feature m: phone Something is referring to the phone module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add phone.number generation options
4 participants