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

refactor(location): use postcode fake patterns #3233

Draft
wants to merge 3 commits into
base: deprecate/location/zipCode/format
Choose a base branch
from

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Oct 29, 2024

Second part of #2390

Blocked by #3223


Replaces the zipcode symbol patterns with fake patterns similar to when using the state parameter.

Locale Data Transformation Script

The locale data have been transformed using the following script:

// rename.ts
import { glob } from 'glob';
import { rmSync, writeFileSync } from 'node:fs';
import { faker } from './src';

const files = await glob('src/locales/*/location/postcode.ts');

for (const file of files) {
  console.log(file);
  let { default: content } = (await import(`./${file}`)) as {
    default: string | string[] | null;
  };

  rmSync(file, { force: true });

  if (typeof content === 'string') {
    content = [content];
  }

  if (content !== null) {
    content = content.map((pattern) =>
      pattern
        .replaceAll(/(#+)/g, ({ length }) =>
          length === 1 ? '{{string.numeric}}' : `{{string.numeric(${length})}}`
        )
        .replaceAll(/(\?+)/g, ({ length }) =>
          length === 1
            ? '{{string.alpha({ "casing": "upper" })}}'
            : `{{string.alpha({ "casing": "upper" , "length": ${length}})}}`
        )
        .replaceAll(/(\*+)/g, ({ length }) =>
          length === 1
            ? '{{string.alphanumeric({ "casing": "upper" })}}'
            : `{{string.alphanumeric({ "casing": "upper" , "length": ${length}})}}`
        )
    );
    for (const pattern of content) {
      console.log(faker.helpers.fake(pattern));
    }
  }

  writeFileSync(
    file.replace('.ts', '_pattern.ts'),
    `export default ${JSON.stringify(content, null, 2)};`
  );
}

pnpm tsx rename.ts followed by pnpm run generate:locales && pnpm run format

@ST-DDT ST-DDT added s: on hold Blocked by something or frozen to avoid conflicts p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs c: locale Permutes locale definitions labels Oct 29, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Oct 29, 2024
@ST-DDT ST-DDT self-assigned this Oct 29, 2024
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (63a4817) to head (b9930dc).
Report is 36 commits behind head on deprecate/location/zipCode/format.

Additional details and impacted files
@@                         Coverage Diff                         @@
##           deprecate/location/zipCode/format    #3233    +/-   ##
===================================================================
  Coverage                              99.96%   99.96%            
===================================================================
  Files                                   2805     2805            
  Lines                                 217110   217143    +33     
  Branches                                 587      969   +382     
===================================================================
+ Hits                                  217028   217066    +38     
+ Misses                                    82       77     -5     
Files with missing lines Coverage Δ
src/definitions/location.ts 100.00% <ø> (ø)
src/locales/af_ZA/location/index.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/location/postcode_pattern.ts 100.00% <100.00%> (ø)
src/locales/ar/location/index.ts 100.00% <100.00%> (ø)
src/locales/ar/location/postcode_pattern.ts 100.00% <100.00%> (ø)
src/locales/az/location/index.ts 100.00% <100.00%> (ø)
src/locales/az/location/postcode_pattern.ts 100.00% <100.00%> (ø)
src/locales/cs_CZ/location/index.ts 100.00% <100.00%> (ø)
src/locales/cs_CZ/location/postcode_pattern.ts 100.00% <100.00%> (ø)
src/locales/da/location/index.ts 100.00% <100.00%> (ø)
... and 114 more

... and 1 file with indirect coverage changes

@matthewmayer
Copy link
Contributor

A lot of these seem very verbose compared to what they are replacing.

In some cases would it make sense to just keep the fake pattern as '{{helpers.replaceSymbols(old pattern}}'

Some could also be simplified by fromRegexp or arrayElement

If we are renaming all the definitions should be change it to zipcode instead of postcode? It's weird that the definition name is different to method name.

@ST-DDT
Copy link
Member Author

ST-DDT commented Oct 30, 2024

I can switch back to replaceSymbol syntax no problem.

One reason for the change is, I don't like that method/it's format because it isn't obvious what the symbol represents.
IMO the code is now easier to read and it would even by easier to read, when/if we switch to resolver functions

In the future, I'd like to deprecate the replaceSymbol methods in favor of regex based methods.
I didn't use regex based implementations yet, because it is too slow for now.

@ST-DDT
Copy link
Member Author

ST-DDT commented Oct 30, 2024

If we are renaming all the definitions should be change it to zipcode instead of postcode? It's weird that the definition name is different to method name.

I'm not sure about the renaming myself yet either.
Should we rename the method or the locale definitions?

@xDivisionByZerox xDivisionByZerox modified the milestones: vAnytime, v10.0 Nov 30, 2024
@xDivisionByZerox xDivisionByZerox added the breaking change Cannot be merged when next version is not a major release label Nov 30, 2024
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: locale Permutes locale definitions c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs p: 1-normal Nothing urgent s: on hold Blocked by something or frozen to avoid conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants