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

Load locales from static files and only bundle once #830

Open
Shinigami92 opened this issue Apr 10, 2022 · 8 comments
Open

Load locales from static files and only bundle once #830

Shinigami92 opened this issue Apr 10, 2022 · 8 comments
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: on hold Blocked by something or frozen to avoid conflicts
Milestone

Comments

@Shinigami92
Copy link
Member

Clear and concise description of the problem

Currently locales are bundled from the src code to dist/esm and dist/cjs. This doubles the needed bundle-size by 2x as much as already needed.

Suggested solution

We should move all locales outside the src code/folder into its own locales folder and bundle them once into dist/locales (minified).
Also we should use another format that is static like json or yaml.
The source locale files can be in a human readable format and they just need to be converted into a best minifyable format possible to minimize the bundle-size of the package.
Then code running from esm or cjs need to access somehow the content of dist/locales. Maybe possible via node:fs?
It might also be that we want to define schema for the locales, so we can validate that no invalid data slip into the locales, checked by CI.

Alternative

No response

Additional context

Other issues like #751 and #823 should be done before starting with this issue.
I just opened this issue so it's transparent where I would like to go in long-time-goal.

@Shinigami92 Shinigami92 added c: feature Request for new feature p: 1-normal Nothing urgent labels Apr 10, 2022
@Shinigami92 Shinigami92 added this to the vFuture milestone Apr 10, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Apr 10, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Apr 10, 2022

This may conflict with possible changes to lazyly loaded locale data.

@Shinigami92
Copy link
Member Author

This may conflict with possible changes to lazyly loaded locale data.

Either I didn't fully understand what you are meaning by this, or I think no, due to the content of the locales would be loaded eagerly as they are loaded now.
Only how they are loaded from static files instead of import statements would change.

@ST-DDT
Copy link
Member

ST-DDT commented Apr 11, 2022

This may conflict with possible changes to lazyly loaded locale data.

Either I didn't fully understand what you are meaning by this, or I think no, due to the content of the locales would be loaded eagerly as they are loaded now. Only how they are loaded from static files instead of import statements would change.

For now, this doesn't have any immediate negative impact. But we have to keep in mind that this prevents us from loading the locale data lazily/dynamically in the future.

So at least the top locale (en) and module files (address) should remain ts files. The module entry files (address.city_name) can be static.

@Shinigami92
Copy link
Member Author

Theoretically this issue could be extended to load locales from anywhere via an abstraction, so it could be loaded from a file, a DB, an web-endpoint or anything else. Only the schema needs to be fulfilled or the data cannot be correctly accesses by faker.

But first I will aim for moving our existing files into static locales as planned, so the bundle size can be halved in size.

@ST-DDT
Copy link
Member

ST-DDT commented Apr 12, 2022

This should be at least a bit flexible to allow for constructs like these:

export first_name = [...import(female_first_name), ... import(male_first_name)].sort().unique();

I'm not sure but maybe it is possible to do something like this:

if (import(moduleEntry).isDynamic) {
    // generate dynamic entrypoint
} else {
    saveAsJSON(import(moduleEntry).default);
}

(only for module entries, not their nested children)

@xDivisionByZerox
Copy link
Member

This issue has been open for long time now. Is this still a relevant issue to discuss?

A concern with static files, which I have not seen being raised so far, is the potential lose of type safety. With file types like json we need to implement some kind of "custom" type validation during build (?) time. 🤔

@ST-DDT
Copy link
Member

ST-DDT commented May 5, 2024

I would like to keep this on hold until the standalone module function rewrites are done and maybe even till the nano bound functions are implemented.

@ST-DDT
Copy link
Member

ST-DDT commented May 5, 2024

Also this looses relevance once we drop CJS support in v10.

@xDivisionByZerox xDivisionByZerox added the s: on hold Blocked by something or frozen to avoid conflicts label May 5, 2024
@ST-DDT ST-DDT modified the milestones: vFuture, v10.0 Oct 20, 2024
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 p: 1-normal Nothing urgent s: on hold Blocked by something or frozen to avoid conflicts
Projects
No open projects
Status: Todo
Development

No branches or pull requests

3 participants