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

Investigate why importing anything from '@faker-js/faker' directly causes all of faker to be bundled. #1791

Closed
ST-DDT opened this issue Jan 29, 2023 · 11 comments · Fixed by #2790
Assignees
Labels
c: bug Something isn't working has workaround Workaround provided or linked help wanted Extra attention is needed p: 2-high Fix main branch
Milestone

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Jan 29, 2023

Clear and concise description of the problem

Importing anything directly from @faker-js/faker causes the bundler to contain all of faker in the resulting bundle.

Reproduction:

https://github.com/faker-js/playground/blob/main/playgrounds/vite-cjs/src/App.vue

Add:

import { FakerError } from "@faker-js/faker";

const natureImageUrl = ref(faker.image.nature() + new FakerError("test"));

That will increase the bundle size from 0.6MB to 3.6MB.

Maybe try with sideEffect: false + refactorings.

Alternative

No response

Additional context

No response

@ST-DDT ST-DDT added c: bug Something isn't working help wanted Extra attention is needed p: 2-high Fix main branch labels Jan 29, 2023
@ST-DDT ST-DDT added this to the v8.0 - Module Re-Shuffling milestone Jan 29, 2023
@ST-DDT ST-DDT moved this to Todo in Faker Roadmap Jan 29, 2023
@Shinigami92
Copy link
Member

So to explain what a side effect is:

When a function modifies the state of something outside its scope, then it is declared as a side effect

For example:

store[result] = result;

store = GLOBAL_UNIQUE_STORE,

const GLOBAL_UNIQUE_STORE: Record<RecordKey, RecordKey> = {};

The helpers.unique function takes GLOBAL_UNIQUE_STORE as a default value and this is defined out of scope of the function itself, but is modified inside it in line 144.
So if the default is taken for unique, then the modification to GLOBAL_UNIQUE_STORE is a side effect.


I'm not aware of any other side effects right now

@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 31, 2023

Would moving this to a class field fix that issue?

NoelDeMartin added a commit to NoelDeMartin/faker that referenced this issue Mar 5, 2023
@ST-DDT
Copy link
Member Author

ST-DDT commented Apr 13, 2023

Team Decision

We will try to investigate this for v8.0

  • Remove global unique store in favor of a faker (helperModule) instance bound one

@Shinigami92
Copy link
Member

@xDivisionByZerox pointed made a hint in meeting 2023-04-20 that we could move the variable allLocales into the file src/locales/index.ts itself

export * as allLocales from './locales';

This should be tested and could work, so that when loading the file, no variable assignment will be made in src/index.ts itself, but only in the underlying file on demand

@Shinigami92
Copy link
Member

Shinigami92 commented Apr 20, 2023

@evanw could you help us with this bug?

Please checkout

git clone git@github.com:faker-js/faker.git
cd faker
pnpm install
pnpm run build

cd ..

git clone git@github.com:faker-js/playground.git
cd playground
git switch next
pnpm install
cd playgrounds/vite-cjs

pnpm run compile # 3759.57 KiB / gzip: 995.54 KiB

Open /playground/playgrounds/vite-cjs/src/App.vue

-   faker.image.urlLoremFlickr({ category: "nature" }) + new FakerError("test")
+   faker.image.urlLoremFlickr({ category: "nature" }) + new Error("test")

run pnpm run compile again, see difference in bundle size (618.70 KiB / gzip: 189.16 KiB)

@Shinigami92 Shinigami92 moved this from Todo to In Progress in Faker Roadmap Apr 20, 2023
@Shinigami92
Copy link
Member

Moving this into Milestone v8 as we have decided that it is not blocking a release v8.0
Still a bug, but we need help e.g. from the community

@graniczny
Copy link

I just realised that faker is taking 3.6MB after build in React...
Imo its a critical issue, its way too big

@xDivisionByZerox
Copy link
Member

Thats why we explicitly ask you to install our library as a Dev Dependency: https://fakerjs.dev/guide/#installation

Install it as a Dev Dependency using your favorite package manager.

There is also additional hints about this in the Usage Guide for the Browser: https://fakerjs.dev/guide/usage.html#browser

Note: Using the browser is great for experimenting 👍. However, due to all of the strings Faker uses to generate fake data, Faker is a large package. It's > 5 MiB minified. Please avoid deploying the full Faker in your web app.

@FredericHeem
Copy link

Faker version 5 is totally fine on the browser.

@ST-DDT
Copy link
Member Author

ST-DDT commented Jun 29, 2023

You can also import a specific locale to avoid the bug.

import { faker } from '@faker-js/faker/locale/de';

https://fakerjs.dev/guide/localization.html#individual-localized-packages

@SalahAdDin
Copy link

Faker version 5 is totally fine on the browser.

It is super heavy.

@ST-DDT ST-DDT added the has workaround Workaround provided or linked label Apr 7, 2024
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 has workaround Workaround provided or linked help wanted Extra attention is needed p: 2-high Fix main branch
Projects
No open projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

6 participants