Skip to content

Add UI module translations to Modrinth App#5489

Open
Jerozgen wants to merge 3 commits intomodrinth:mainfrom
Jerozgen:ui-locale-in-app
Open

Add UI module translations to Modrinth App#5489
Jerozgen wants to merge 3 commits intomodrinth:mainfrom
Jerozgen:ui-locale-in-app

Conversation

@Jerozgen
Copy link
Contributor

@Jerozgen Jerozgen commented Mar 6, 2026

No description provided.

@IMB11
Copy link
Member

IMB11 commented Mar 6, 2026

Hi, in your PRs, are you using the pnpm prepr:frontend command(s) to lint/format your changes? I'm seeing some changes here which aren't relevant to your PR which might be caused by an IDE or a different formatter?

@Jerozgen
Copy link
Contributor Author

Jerozgen commented Mar 6, 2026

Yea, I use prepr:frontend

@Jerozgen
Copy link
Contributor Author

Jerozgen commented Mar 6, 2026

The prepr actually fails on src\modules\labrinth\types.ts because team_members is any[], so I comment it, run the prepr, then uncomment

@IMB11
Copy link
Member

IMB11 commented Mar 6, 2026

Would you mind quickly fixing that in another PR? Set it to unknown[]?

@Jerozgen Jerozgen mentioned this pull request Mar 6, 2026
Merged
IMB11
IMB11 reviewed Mar 6, 2026
View reviewed changes
packages/ui/src/locales.eager.ts

export const uiLocaleModulesEager = import.meta.glob<{ default: CrowdinMessages }>(
'./locales/*/index.json',
{ eager: true },
Copy link
Member

@IMB11 IMB11 Mar 6, 2026

Choose a reason for hiding this comment

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

This damages performance, should not be using eager at all :)

Copy link
Contributor Author

@Jerozgen Jerozgen Mar 7, 2026

Choose a reason for hiding this comment

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

The eager: true is used on the app-frontend module translations, so I've also used it on the ui translations. For the website, the ui translations are still loaded with eager: false

Do you want me to rewrite it in a similar way to the website i18n context injection?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IMB11 IMB11 IMB11 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Jerozgen @IMB11