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

Add lazy loading of lang files #1310

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

geichelberger
Copy link
Contributor

@geichelberger geichelberger commented Apr 4, 2024

Only the currently used language is loaded instead of all language files from the beginning. A new lazy loading backend loads the lang files as a dynamic import.

This should fix the bug that sometimes the language keys are missing.
grafik

@geichelberger geichelberger added the type:bug Something isn't working label Apr 4, 2024
Copy link

github-actions bot commented Apr 4, 2024

This pull request is deployed at test.editor.opencast.org/1310/2024-04-04_07-29-29/ .
It might take a few minutes for it to become available.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Works. Can't really confirm whether this fixes the issue with languages not loading sometimes as I cannot consistently reproduce it, but at the very least it does not break anything obvious :D

That being said, does renaming the locales break our crowdin workflow?

src/i18n/config.tsx Outdated Show resolved Hide resolved
@geichelberger
Copy link
Contributor Author

geichelberger commented Apr 4, 2024

That being said, does renaming the locales break our crowdin workflow?

I missed that the locales.json is generated automatically; this also resolves the issues of populating the array. I will change that.

Copy link

github-actions bot commented Apr 4, 2024

This pull request is deployed at test.editor.opencast.org/1310/2024-04-04_11-21-29/ .
It might take a few minutes for it to become available.

@geichelberger
Copy link
Contributor Author

I changed how the crowdin translation array/map is generated by replacing the locales.json with an lngs-generated.ts file. The locales now have the same name as before, and the generation script creates a map of the country codes and the languages.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Can't really test this, but in general this looks solid to me (if maybe a little convoluted, but that's more i18ns fault than anything).

Copy link

github-actions bot commented Apr 8, 2024

This pull request is deployed at test.editor.opencast.org/1310/2024-04-08_18-20-40/ .
It might take a few minutes for it to become available.

@geichelberger
Copy link
Contributor Author

I changed the script path to use the absolute path with $GITHUB_WORKFLOW because the action uses a working directory.

@github-actions github-actions bot added the status:conflicts Conflicts with another pull request or issue label Apr 16, 2024
Copy link

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@github-actions github-actions bot removed the status:conflicts Conflicts with another pull request or issue label Apr 16, 2024
Copy link

This pull request is deployed at test.editor.opencast.org/1310/2024-04-16_14-20-21/ .
It might take a few minutes for it to become available.

Only the currently used language is loaded instead of all language files
from the beginning. A new lazy loading backend loads the lang files as a
dynamic import.
Copy link

This pull request is deployed at test.editor.opencast.org/1310/2024-04-26_11-47-56/ .
It might take a few minutes for it to become available.

@KatrinIhler
Copy link
Member

Discussed and approved, change sounds reasonable -> merging.

@KatrinIhler KatrinIhler merged commit 4c4501e into opencast:main Apr 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants