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: de-proxy locale access #3189

Open
wants to merge 7 commits into
base: feat/samfn/faker-core
Choose a base branch
from

Conversation

ST-DDT
Copy link
Member

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

Second part of the standalone module function feature #2667

Fixes #3155

Requires #2838

Overview

  1. Introduce FakerCore feat: introduce FakerCore #2838
  2. De-proxy locale access feat: de-proxy locale access #3189
  3. De-class config function #future (seed, setDefaultRefDate, ...)
  4. De-module functions #future (airline, ...)

Description

Instead of calling this.faker.definitions.category.entry, we will now use resolveLocaleData(this.faker.fakerCore, 'category', 'entry').
Please note that this will get simplified with the next PR as this.faker.fakerCore will get replaced with just fakerCore.
This change makes the functions independent from the main faker instance, at least locale access wise.
That makes resolveLocaleData the first effective "standalone module function", as it no longer needs faker, only the fakerCore is needed.

As an additional benefit it also fixes #3155

Recreating the PR

  • Cherry-pick the first and second commit.
  • Replace this.faker\.definitions\.(\w+)\.(\w+) with resolveLocaleData(this.faker.fakerCore, '$1', '$2') in src.
  • Replace faker\.definitions\.(\w+)\.(\w+) with resolveLocaleData(faker.fakerCore, '$1', '$2') in src.
  • Replace faker\.definitions\.(\w+)\??\.(\w+) with resolveLocaleData(faker.fakerCore, '$1', '$2') in test.
  • Replace faker\.definitions\.(\w+)\[(\w+)\] with resolveLocaleData(faker.fakerCore, '$1', $2) in test.
  • Add missing imports
  • Revert faker.spec.ts
  • Cherry-pick the manual patches.

Next Steps (Future PR)

The next step after this PR is converting the other methods to standalone module functions.

Current

export class AirlineModule extends ModuleBase {
  airport(): Airport {
    return this.faker.helpers.arrayElement(
      resolveLocaleData(this.faker.fakerCore, 'airline', 'airport')
    );
  }
}

Future

function airport(fakerCore: FakerCore): Airport {
  return arrayElement(fakerCore, 
    resolveLocaleData(fakerCore, 'airline', 'airport')
  );
}

export class AirlineModule extends ModuleBase {
  airport(): Airport {
    // return airport(this.faker.fakerCore);
    return airport(this.fakerCore);
  }
}

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent do NOT merge yet Do not merge this PR into the target branch yet labels Oct 16, 2024
@ST-DDT ST-DDT added this to the v10.0 milestone Oct 16, 2024
@ST-DDT ST-DDT self-assigned this Oct 16, 2024
@ST-DDT ST-DDT requested a review from a team as a code owner October 16, 2024 23:58
@ST-DDT ST-DDT marked this pull request as draft October 17, 2024 00:00
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 98.24561% with 5 lines in your changes missing coverage. Please review.

Project coverage is 99.96%. Comparing base (e659674) to head (c67f876).

Files with missing lines Patch % Lines
src/modules/person/index.ts 82.14% 5 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           feat/samfn/faker-core    #3189      +/-   ##
=========================================================
- Coverage                  99.97%   99.96%   -0.01%     
=========================================================
  Files                       2800     2801       +1     
  Lines                     227385   227531     +146     
  Branches                     580      960     +380     
=========================================================
+ Hits                      227319   227461     +142     
- Misses                        66       70       +4     
Files with missing lines Coverage Δ
src/definitions/definitions.ts 100.00% <ø> (ø)
src/faker.ts 100.00% <ø> (ø)
src/index.ts 100.00% <ø> (ø)
src/internal/locale-proxy.ts 100.00% <100.00%> (ø)
src/modules/airline/index.ts 100.00% <100.00%> (ø)
src/modules/animal/index.ts 100.00% <100.00%> (ø)
src/modules/book/index.ts 100.00% <100.00%> (ø)
src/modules/color/index.ts 100.00% <100.00%> (ø)
src/modules/commerce/index.ts 97.75% <100.00%> (+0.03%) ⬆️
src/modules/company/index.ts 100.00% <100.00%> (ø)
... and 18 more

... and 1 file with indirect coverage changes

@ST-DDT ST-DDT requested a review from a team October 17, 2024 11:12
@ST-DDT ST-DDT marked this pull request as ready for review October 17, 2024 11:12
@ST-DDT ST-DDT linked an issue Oct 18, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature do NOT merge yet Do not merge this PR into the target branch yet p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support older browsers without Proxy
1 participant