fix: improve language selection resolution #1011
Draft
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.
Fixes #978
The issues
There were a few issues I noticed about how we handle language settings that were prone to bugs:
We immediately persist the initial state of a persisted zustand store. It wasn't apparent to me at the time of implementation, but this is not a desirable behavior. Persistence should generally happen based on explicitly actions or via some kind of clearly defined effect. Doing this implicitly contributes to the issue that is detailed next.
System preferences could be ignored unintentionally. Basically what's highlighted in App does not use system language preference when expected #978, but here's a sequence that highlights the various issues:
Steps (4) and (5) highlight a couple of the issues:
The main problem here lies in the first issue I mentioned. Using a more code-friendly explanation, what's happening is the following:
'en'
as the language to use for the app.'en'
is persisted to storage because of issue 1'pt'
, but our calculation of the language to use is not reactive to this, as we only check the system settings when initializing Zustand (usinggetLocales()
fromexpo-localization
) and only in the case that something is not already persisted'en'
even though it should be'pt'
Terminology
Before getting into the changes that address the issues above, it's helpful to clarify some wording that I've committed to for this PR. I found it confusing to figure out what a "locale" meant versus a "language code" versus a "language tag" when it came to what's represented in code, so I made a concerted effort to distinguish between them appropriately:
locale: kind of this nebulous definition of a language based on many components that define how a language is presented and used. basically, the
Locale
type fromexpo-localization
is a good example. given this, I found our usage of the wordlocale
in the codebase to be a bit too vague (and somewhat misleading), so I made lots of efforts in the PR to avoid using it for naming where possible.language tag: basically what's detailed here. These can either be single or multi-component strings, where each components adds a degree of specificity (e.g. regional code). For example,
en
anden-US
are both language tags.language code: in practice, it's the component of a language tag that defines the primary language, which is typically the first component of the language tag (i.e. before the first hyphen if there are any). For example, the language code for
en-US
isen
. For all intents and purposes of our usage, every language code is a valid language tag.Changes this PR introduces
uses our new approach to Zustand-based state for persisting a selected locale. What's persisted is information related to language that is explicitly selected via user action (e.g. a store action). In this case, the storage looks like
{ languageCode: string | null }
. There is no more implicit persistence on initialization like before.introduces a hook called
useResolvedLanguageTag()
which figures out which language tag to use for the app based on:This hook solves the issue where we aren't reactively updating based on changes to the above. In addition to that, it also accounts for multiple system preferences. Before, we'd only take the first language preference and use that, but that's prone to unideal behavior because a user could have a set of preferred languages that may not actually be supported by the app. For example, if the system preferences were something like
["foo", "bar", "pt"]
, the existing behavior would attempt to only use"foo"
despite"pt"
being available and supported in the app.cleans up the implementation for intl-related utilities and resolution of language tags. I noticed that there's a noticeable distinction between the following:
messages/
)languages.json
file.Using this categorization, the
useResolvedLanguageTag()
hook basically returns the best fitting "usable" languageSome implementation concerns/questions
Use existing storage key or not?
As I refactored the locale state setup, I was trying to decide between two options:
usePersistedLocale()
hook implementation)At the time of writing this, I've currently chosen (1) for the sake of backwards compatibility. (2) would be easier and less work, but in the worst case, it would mean that anyone who did explicitly select a language before would lose that selection. The app would be presented based on the system preference or the English fallback. Personally I don't find this to be a major issue but I can imagine it being quite surprising, especially for low tech users.
In choosing (1), I've updated the shape of the storage to be more forwards compatible. There's a zustand migration that i've implemented to migrate from version 0 (the default version associated with previous implementation) and version 1 (this new implementation). You may think this migration implementation is unnecessary work. In terms of what's stored, that's a valid criticism, but I would argue that we should be getting in the habit of versioning all of our persisted stores and implementing migrations where appropriate (such is the consequence of offline + local first!)
I'm not too sure what the best way to test the migrations is, as it ideally requires having the previous storage version persisted and then updating the app to use the new one in order to trigger the migration. Open to thoughts on how to go about that in a somewhat reliable and reproducible way.
Example of the app responding to changes in system preferences (no explicit choosing of language in in-app settings):
system-language-switch.mp4