Skip to content

Add lazy loading of lang files#1310

Merged
KatrinIhler merged 3 commits into
opencast:mainfrom
geichelberger:i18n-lazy-loading
Apr 26, 2024
Merged

Add lazy loading of lang files#1310
KatrinIhler merged 3 commits into
opencast:mainfrom
geichelberger:i18n-lazy-loading

Conversation

@geichelberger
Copy link
Copy Markdown
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
@github-actions
Copy link
Copy Markdown

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
Copy Markdown
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?

Comment thread src/i18n/config.tsx Outdated
@geichelberger
Copy link
Copy Markdown
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.

@github-actions
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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).

@github-actions
Copy link
Copy Markdown

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
Copy Markdown
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
@github-actions
Copy link
Copy Markdown

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
@github-actions
Copy link
Copy Markdown

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.
@github-actions
Copy link
Copy Markdown

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
Copy Markdown
Member

Discussed and approved, change sounds reasonable -> merging.

@KatrinIhler KatrinIhler merged commit 4c4501e into opencast:main Apr 26, 2024
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.

3 participants