fluent.js

fluent.js copied to clipboard

Can we have setL10n in useLocalization so we can easily set the new value in a nested component?

Thanks for filing the issue! I've been thinking about something similar: let {l10n, changeLanguages} = useLocalization(); but I think setL10n might be easier. @macabeus What do you think?

I'm thinking... And yeah, maybe it could be useful.

On the client side, using the example from fluent-react, on l10n.tsx we'll need to something like:

- <LocalizationProvider l10n={l10n}>
+ <LocalizationProvider l10n={l10n} setL10n={changeLocales}>

Then we'll be able to use const { setL10n } = useLocalization() anywhere. It could be useful if you want to create one or more components on deeper level to change the language. Nice.

Just adding the prop setL10n on <LocalizationProvider /> and updating the value of FluentContext from l10n to an object { l10n, setL10n } is the most simple implementation. But let’s brainstorm. Maybe we can to do something even better:

Currently setL10n receives as parameter an array. Is it make sense on the use case from a hook? Maybe a string could be simpler and fit better (setL10n('pt') instead of setL10n(['pt']))? Also, if you set an unknown language (setL10n(['blaah'])), it'll fails silently... is it the best behaviour on this case?

Also and most important IMHO... Currently is necessary to write a big boilerplate to use Fluent. l10n.ts has 78 lines! I think that we could improve that setting a default setL10n function on LocalizationProvider. So the client won't need to write again a setL10n function and pass it to <LocalizationProvider /> - only if they want to use a custom setL10n, of course.

What do you think about that, @stasm ?

Just adding the prop setL10n on <LocalizationProvider /> and updating the value of FluentContext from l10n to an object { l10n, setL10n } is the most simple implementation.

Yeah, I was thinking along the same lines. That's why I liked your idea in #475 to return a plain {l10n} object from the hook.

Maybe a string could be simpler and fit better (setL10n('pt') instead of setL10n(['pt']))?

In general, I'd like the API to be aligned with the navigator.languages API, which means working with arrays of language codes. This way the user's preference can be expressed as a list of languages rather than a single language.

That said, I guess the primary use-case of setL10n/changeLocales will be to force-change the language of the app using the UI, e.g. a dropdown with the list of available languages. In this use-case, a single string parameter could be sufficient. We'd need to consider other possible use-cases and consistency with the rest of @fluent/react's API.

Currently is necessary to write a big boilerplate to use Fluent. l10n.ts has 78 lines! I think that we could improve that setting a default setL10n function on LocalizationProvider

I think most of that boilerplate is needed and in fact is what makes @fluent/react flexible and agnostic wrt. where and how the translations are stored as well as wrt. the language negotation. I wrote about it at https://github.com/projectfluent/fluent.js/wiki/ReactLocalization#flexibility.

Here's how I imagined changeLocales being used if exposed through the hook:

export function AppLocalizationProvider(props: AppLocalizationProviderProps) {
    let [currentLocales, setCurrentLocales] = useState([DEFAULT_LOCALE]);
    let [l10n, setL10n] = useState<ReactLocalization | null>(null);

    useEffect(() => {
        changeLocales(navigator.languages as Array<string>);
    }, []);

    async function changeLocales(userLocales: Array<string>) {
        // Negotiate user locales × available locales
        let currentLocales = …;
        setCurrentLocales(currentLocales);

        // Fetch translations and instantiate ReactLocalization.
        let bundles = await ...;
        let l10n = new ReactLocalization(bundles);
        setL10n(l10n);
    }

    return <>
        <LocalizationProvider l10n={l10n} changeLocales={changeLocales}>
            {Children.only(props.children)}
        </LocalizationProvider>
    </>;
}

// Somewhere else:
function MyComponent() {
    let {l10n, changeLocales} = useLocalization();
    changeLocales(["fr-CA", "fr"]);
}

If we expose setL10n instead I think it would become harder to manage currentLocales. In the example below, I can call setL10n but it won't have an effect on currentLocales stored in AppLocalizationProvider's state.

async function changeLocales(userLocales: Array<string>) {
    // Negotiate user locales × available locales
    let currentLocales = …;
    // Fetch translations and parse them into FluentBundles.
    let bundles = await ...;
    let l10n = new ReactLocalization(bundles);
    return {currentLocales, l10n};
}

export function AppLocalizationProvider(props: AppLocalizationProviderProps) {
    let [currentLocales, setCurrentLocales] = useState([DEFAULT_LOCALE]);
    let [l10n, setL10n] = useState<ReactLocalization | null>(null);

    useEffect(() => {
        let {currentLocales, l10n} = changeLocales(navigator.languages as Array<string>);
        setCurrentLocales(currentLocales);
        setL10n(l10n);
    }, []);

    return <>
        <LocalizationProvider l10n={l10n} setL10n={setL10n}>
            {Children.only(props.children)}
        </LocalizationProvider>
    </>;
}

// Somewhere else:
function MyComponent() {
    let {l10n, setL10n} = useLocalization();
    let newL10n = changeLocales(["fr-CA", "fr"]);
    setL10n(l10n);
}

To solve this, we could expose setCurrentLocales too, or expose a wrapper which calls both setCurrentLocales and setL10n. Which sounds like changeLocales from my comment above :)

Or, we could store currentLocales inside the ReactLocalization instance, which might be an intersting avenue to consider.

I think that exposing changeLocales on useLocalization is enough for major requirements to be useful on this hook. At the first moment I was thinking that changeLocales and setL10n are the same thing, but seeing your example now I understand that are different meanings. changeLocales looks simpler and easier to use than setL10n.

I like to thinking with code, so I'm opening a PR following your first approach. Then I noticed that in order to this new feature be useful to use on a selector, we'll also need to provide a variable saying the current locale (currentLocale?), otherwise we'll have two sources of truth, that is bad.

I think that, since we'll have both a setter and reader, we could call it following the React's useState: locales and setLocales.

PR with the proof of concept: https://github.com/projectfluent/fluent.js/pull/501

What about 2 hooks? 1 low-level (useLocalization) 1 high-level (useLocales)?