Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Internationalization & localization #1822
Internationalization & localization #1822
Changes from all commits
28d354b
d88ca9a
8013685
7df8543
7b5427a
400d92d
7d0504a
80944a8
a3851b2
b477b3b
713a24a
8531a98
4805e65
f7f62e5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that these pitfalls only matter if we allow changing the locale without reloading the app? I guess that's unavoidable on native?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is correct.
If we use the hook and properly memoize any functions that use data from the hook, then the UI will react to changes in the locale.
When it's implemented in the whole app, I doubt it'll be be instantaneous as the new locale will need to be loaded, then all components will need to be re-rendered. It'll be like switching from light mode -> dark mode, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks concerning to me. What will it be doing while the locale data is loading? Is this blocking? If it's not blocking, how does the UI work in the meantime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can check the
i18n.ts
code -- it's just a dynamic importThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand it's a dynamic import — what I don't understand is what strings will we be showing until the default locale loads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can either show nothing or the default locale (English)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see if any screen flashing becomes an issue...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things to verify on mobile:
Things to verify on web:
At FB I think we'd just create a complete JS bundle per locale and force a full reload on locale change. This is nice because you don't have to worry about locale changing dynamically at all.
I wonder if an approach like this would work for us? My concern with locales as separate files is that they grow unbounded: we'd have to download all strings even if we use code splitting for the actual code (and so the initial bundle doesn't need all strings). I think that's the motivation for replacing content in the bundles instead.
Its docs are not great, but I strongly recommend investigating how https://facebook.github.io/fbt/ handles this. I wonder if the open source version does bundle replacement... Can you check how this works? https://github.com/facebook/fbt/tree/main/demo-app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lingui works very similar to FBT from what I can tell. I'll take a deeper look and then we can adjust the method that we use. However, I do think that not reloading the app on locale change would be a better user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm inclined to say we should bundle the english locale and then dynamically load the preferred locale once we have the preference. It's not ideal but it may be the least worst option - ensures there are strings and keeps the bundle as small as possible.
The calculus is likely different between native and web. Web is the harder one. Caching may help us -- though we also need to ensure dynamic loading works in the prod build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On mobile, everything will be bundled automatically and then switch when we want it to. I expect our locale
messages.js
files to be around20kb
. Right now its14kb
for English.On web, it will be fetched when required. We can cache it and that will definitely help. Here is how it works on web: https://lingui.dev/guides/dynamic-loading-catalogs#conclusion
On RN, we would have to implement our own solution. There has been some prior art on this: lingui/js-lingui#503