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

Update Lingui to 5.1.1 #7138

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

auroursa
Copy link
Contributor

@auroursa auroursa commented Dec 17, 2024

There are some new features here that I'm really interested in, with Print Placeholder Values, this allows us to know exactly what the placeholders are.

Before this, {0} of {1} was difficult to translate.

image

After updates, translators can more easily understand {0} is currentTime and {1} is duration without having to check the source code.

image

The first commit is more like a regular upgrade, with no major changes.

The second commit removed @lingui/macro according to the migration guide and updated all related imports. We can stick to the first commit for now, but since @lingui/macro is deprecated and will be removed in the next major version, we may need to address this later.

EDIT: A patch is no longer necessary, as this issue has been resolved in lingui 4.7.1.

@auroursa
Copy link
Contributor Author

I used @lingui/codemods to run the bulk replacement of imports. Running it based on the first commit should get the same result as my second commit.
image

@timofei-iatsenko
Copy link

Hey, there is also a new useLingui macro and automatic community migration for it which would help transform something like this

 accessibilityLabel: _(msg`Hashtag: #${tag}`),
 accessibilityHint: _(msg`Long press to open tag menu for #${tag}`),

to this

const { t } = useLingui()
//
 accessibilityLabel: t`Hashtag: #${tag}`,
 accessibilityHint: t`Long press to open tag menu for #${tag}`,

@auroursa auroursa marked this pull request as draft December 17, 2024 09:04
@auroursa
Copy link
Contributor Author

This might require a decision from the developers. Currently, there is almost no situation where _ and i18n are used together, but it's quite common to see const { _ } = useLingui() in projects.

If we need to change const { _ } = useLingui() to const { t } = useLingui(), this could conflict with existing variable names, such as const t = useTheme().

@auroursa
Copy link
Contributor Author

@timofei-iatsenko Thank you for your patient assistance, I would like to know if it’s possible to still use useLingui directly from lingui/react and continue with const { _ } = useLingui() as before, or if I must migrate to const { t } = useLingui()?

@timofei-iatsenko
Copy link

@auroursa it is completely optional, so you can leave it as it was before.

@auroursa auroursa marked this pull request as ready for review December 17, 2024 14:53
@auroursa auroursa marked this pull request as draft December 20, 2024 03:12
@auroursa
Copy link
Contributor Author

Considering the issue in #7195, I will wait for support date,time format to be merged before testing the new version of lingui. This will no longer introduce new polyfills and will attempt to resolve #6728 again.

@timofei-iatsenko
Copy link

Considering the issue in #7195, I will wait for support date,time format to be merged before testing the new version of lingui. This will no longer introduce new polyfills and will attempt to resolve #6728 again.

@auroursa did I understand you correctly that you hope that support date,time format will resolve the issue with Chinese and you will not need the polyfills?

@auroursa
Copy link
Contributor Author

@timofei-iatsenko I think it seems to be this way, but if I have misunderstood anything, please feel free to let me know.

@timofei-iatsenko
Copy link

@auroursa that PR just adds new features, they are still based on Intl API, so polyfiils are needed.

@auroursa
Copy link
Contributor Author

Thank you. I will focus this PR on upgrading lingui, and the date format-related issues will be driven forward by reintroducing polyfills.

@timofei-iatsenko
Copy link

nitpick from my side, you probably need to update this documentation https://github.com/bluesky-social/social-app/blob/main/docs/localization.md

@auroursa
Copy link
Contributor Author

Ping @surfdude29 here, I’m not sure if you’d be interested in improving the localization documentation, but perhaps we could work on it together? Not just the Lingui use case, recently there have been some localization PRs complaining that the documentation is a bit outdated.

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