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: handle missing or broken locales main index files #478

Merged
merged 4 commits into from
Feb 14, 2022

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Feb 12, 2022

While preparing #477 I encountered the issue, that I renamed the files, but did not fix the imports (as that is supposed to be done by the generate:locales script instead.

However, the script failed to execute, because it tries to import the locale to read it's title and separator.

$ pnpm run generate:locales

> @faker-js/faker@6.0.0-alpha.6 generate:locales ~/git/Faker/faker
> esno ./scripts/generateLocales.ts

Error: Cannot find module './bicycle'
Require stack:
- ~/git/Faker/faker/src/locales/en/vehicle/index.ts
- ~/git/Faker/faker/src/locales/en/index.ts
- ~/git/Faker/faker/scripts/generateLocales.ts
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (~/git/Faker/faker/src/locales/en/vehicle/index.ts:6:21)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Module._compile (~/git/Faker/faker/node_modules/.pnpm/esbuild-register@3.3.2_esbuild@0.14.21/node_modules/esbuild-register/dist/node.js:2258:26)
    at Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Object.newLoader [as .ts] (~/git/Faker/faker/node_modules/.pnpm/esbuild-register@3.3.2_esbuild@0.14.21/node_modules/esbuild-register/dist/node.js:2262:9)
    at Module.load (node:internal/modules/cjs/loader:981:32) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '~/git/Faker/faker/src/locales/en/vehicle/index.ts',
    '~/git/Faker/faker/src/locales/en/index.ts',
    '~/git/Faker/faker/scripts/generateLocales.ts'
  ]
}

This PR prevents these kind of issues, by adding two fallback layers.

  1. If the module fails to load, it will attempt to manually parse the file.
  2. If that also fails, it will just generate a default value, in an attempt to create at least a working and improvable file.

With this new version it is possible to remove or invalidate all index.ts files from src/locales and regenerate them (without the original title though).

@ST-DDT ST-DDT added c: bug Something isn't working c: feature Request for new feature labels Feb 12, 2022
@ST-DDT ST-DDT requested a review from a team February 12, 2022 00:53
@ST-DDT ST-DDT self-assigned this Feb 12, 2022
@codecov
Copy link

codecov bot commented Feb 12, 2022

Codecov Report

Merging #478 (92b7b1f) into main (829f7ff) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #478      +/-   ##
==========================================
- Coverage   99.34%   99.34%   -0.01%     
==========================================
  Files        1919     1919              
  Lines      176291   176291              
  Branches      898      898              
==========================================
- Hits       175132   175131       -1     
- Misses       1103     1104       +1     
  Partials       56       56              
Impacted Files Coverage Δ
src/helpers.ts 99.39% <0.00%> (-0.16%) ⬇️

@Shinigami92 Shinigami92 added p: 2-high Fix main branch and removed c: feature Request for new feature labels Feb 12, 2022
@Shinigami92 Shinigami92 requested a review from a team February 12, 2022 10:20
@ST-DDT ST-DDT merged commit ff97431 into main Feb 14, 2022
@ST-DDT ST-DDT deleted the fix/generate-locales-with-broken-modules branch February 14, 2022 15:34
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working p: 2-high Fix main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants