Skip to content

chore: move faker into own file #548

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Feb 24, 2022
Merged

chore: move faker into own file #548

merged 9 commits into from
Feb 24, 2022

Conversation

Shinigami92
Copy link
Member

@Shinigami92 Shinigami92 added the c: chore PR that doesn't affect the runtime behavior label Feb 23, 2022
@Shinigami92 Shinigami92 self-assigned this Feb 23, 2022
@Shinigami92
Copy link
Member Author

@ST-DDT Could you please have a look if this is what you expected?
Also please have a look and jump into this PR if anything is needed for documentation (automation script and so on)

@codecov
Copy link

codecov bot commented Feb 23, 2022 •
edited
Loading

Codecov Report

Merging #548 (99ff211) into main (6c9dcdd) will decrease coverage by 0.00%.
The diff coverage is 72.19%.

❗ Current head 99ff211 differs from pull request most recent head 15e0284. Consider uploading reports for the commit 15e0284 to get more accurate results

@@            Coverage Diff             @@
##             main     #548      +/-   ##
==========================================
- Coverage   99.34%   99.34%   -0.01%     
==========================================
  Files        1919     1920       +1     
  Lines      176364   176368       +4     
  Branches      905      906       +1     
==========================================
+ Hits       175210   175213       +3     
- Misses       1098     1099       +1     
  Partials       56       56
Impacted Files Coverage Δ
src/locale/af_ZA.ts 0.00% <0.00%> (ø)
src/locale/ar.ts 0.00% <0.00%> (ø)
src/locale/az.ts 0.00% <0.00%> (ø)
src/locale/cz.ts 0.00% <0.00%> (ø)
src/locale/de.ts 0.00% <0.00%> (ø)
src/locale/de_AT.ts 0.00% <0.00%> (ø)
src/locale/de_CH.ts 0.00% <0.00%> (ø)
src/locale/el.ts 0.00% <0.00%> (ø)
src/locale/en.ts 0.00% <0.00%> (ø)
src/locale/en_AU.ts 0.00% <0.00%> (ø)
... and 48 more

ST-DDT
ST-DDT reviewed Feb 23, 2022
View reviewed changes
@ST-DDT
Copy link
Member

ST-DDT commented Feb 23, 2022

Also please have a look and jump into this PR if anything is needed for documentation (automation script and so on)

Will do so this evening.

@Shinigami92 Shinigami92 marked this pull request as ready for review February 23, 2022 21:18
@Shinigami92 Shinigami92 requested a review from a team as a code owner February 23, 2022 21:18
@Shinigami92 Shinigami92 requested a review from ST-DDT February 23, 2022 21:18
@Shinigami92 Shinigami92 mentioned this pull request Feb 23, 2022
Merged
@Shinigami92
Copy link
Member Author

Please ignore the codecov/patch job for this PR

@ST-DDT ST-DDT requested a review from a team February 23, 2022 23:01
@Shinigami92 Shinigami92 added p: 3-urgent Fix and release ASAP and removed c: chore PR that doesn't affect the runtime behavior labels Feb 24, 2022
JessicaSachs
JessicaSachs requested changes Feb 24, 2022
View reviewed changes
Copy link
Contributor

@JessicaSachs JessicaSachs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This doesn't fix the size issues yet. pnpm build for a Vite project that only does

import faker from '@faker-js/faker/locale/en'
console.log(faker.locales)

is still the same bundle size as a project that loads all of Faker. See pics of before + after sizes.

Screen Shot 2022-02-24 at 1 34 55 PM

Screen Shot 2022-02-24 at 1 35 16 PM

@Shinigami92
Copy link
Member Author

Please have a look into the dist/esm folder and lock how many chunks are generated

@JessicaSachs JessicaSachs self-requested a review February 24, 2022 18:50
JessicaSachs
JessicaSachs approved these changes Feb 24, 2022
View reviewed changes
Copy link
Contributor

@JessicaSachs JessicaSachs left a comment

Choose a reason for hiding this comment

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

Ahhh. Had to rerun. It works perfectly. Down 500kb minified + gzipped.
Screen Shot 2022-02-24 at 1 50 38 PM
.

@Shinigami92 Shinigami92 merged commit 51e2486 into main Feb 24, 2022
@Shinigami92 Shinigami92 deleted the move-faker-into-own-file branch February 24, 2022 19:07
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
@Shinigami92 @ST-DDT @demipel8
chore: move faker into own file (faker-js#548)
2b5f993 
Co-authored-by: ST-DDT <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@JessicaSachs JessicaSachs JessicaSachs approved these changes

Assignees

@Shinigami92 Shinigami92

Labels
p: 3-urgent Fix and release ASAP
Projects
None yet
Milestone
v6.0 - Ensure Project Foundation
Development

Successfully merging this pull request may close these issues.
3 participants
@Shinigami92 @ST-DDT @JessicaSachs