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: handle scoped only libs #179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aymeric-duchein
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: 178
Issue Number: 132

As stated in the first linked issue, when using libraries that only need scoped translation you end up with unused files generated because of the "__global" scope.

What is the new behavior?

Using a new scopedOnly option in the config file, we can now tell the builder and finder, to only check for the files in the scopePathMap and ignore everything in the __global scope. If you have the emitErrorOnExtraKeys option enabled you''l get an error if the key builder detects global keys, only a warning otherwise.
With this options, you should now be able to have only one translation file per library per lang and, if you want to, have this file at the root path (rootTranslationsPath option)
Also updated the marker typing according to the second issue.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I'll add tests and documentation if you feel the PR is acceptable 😄

@aymeric-duchein
Copy link
Author

Hi again! Did you had any time to check this ?

Copy link
Collaborator

@shaharkazaz shaharkazaz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! sorry it took me a while to get to it

'Global keys found with scopedOnly flag active\n'
);
if (config.emitErrorOnExtraKeys) {
process.exit(2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why exit code 2? 🤔

process.exit(2);
}
}
delete (scopeToKeys as Partial<ScopeMap>).__global;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to delete the key?

Comment on lines +32 to +44
if (config.scopedOnly) {
if (Object.keys(result.scopeToKeys.__global).length) {
logger.log(
'\n\x1b[31m%s\x1b[0m',
'⚠️',
'Global keys found with scopedOnly flag active\n'
);
if (config.emitErrorOnExtraKeys) {
process.exit(2);
}
}
delete (result.scopeToKeys as Partial<ScopeMap>).__global;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicate code from the builder

const globalFiles = langs.map((lang) => ({
path: `${output}/${lang}.${fileFormat}`,
}));
for (const { path } of globalFiles) {
Copy link
Collaborator

@shaharkazaz shaharkazaz Apr 13, 2024

Choose a reason for hiding this comment

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

Generally speaking, shouldn't we just skip creation if there aren't any keys there? instead of introducing a new config? or maybe introduce "createEmptyFiles" or something, WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants