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

Islands example #246

Merged
merged 20 commits into from
Oct 12, 2024
Merged

Islands example #246

merged 20 commits into from
Oct 12, 2024

Conversation

NCura
Copy link
Contributor

@NCura NCura commented Oct 2, 2024

Hello,

This is my current implementation of an islands example. I've started with the Axum template, integrated the islands feature, and added the leptos_fluent dependencies. In app.rs, I introduced a macro to maintain consistent arguments across the application, which is called within the App() function.

To better reflect a real-world scenario, I created a new module instead of keeping everything in a single file. The module includes a HomePage() component, which utilizes the macro_tr!. Since it's a #[component], it automatically has access to the i18n context.

There are also two islands, each requiring its own i18n context. I modified the LanguageSelector to use the i18n context that was explicitly created, rather than relying on shortcuts that assume the context is already available.

The current setup is functional, but I see a few areas for improvement:

  • I'm currently reloading the page after changing the language because, without the reload, the changes don't propagate. It seems that the i18n context in LanguageSelector differs from the one in Counter. The language change needs to be broadcasted to all other parts of the site where translations are used, such as Counter, HomePage, and even App for updating the Title.
  • I'm unclear on why [TRANSLATIONS, TRANSLATIONS] + COMPOUND is being used instead of just [TRANSLATIONS], as demonstrated in the translations example from the Leptos Fluent documentation.
  • Could we simplify the i18n! macro to take no parameters, embedding the Translations array directly within the macro rules? This would eliminate the need to invoke static_loader! in every file that uses translations.

I'm happy to make further changes based on any suggestions or feedback!

Copy link
Owner

@mondeja mondeja left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR.

To make the example more real, please, use a separate translations directory for the "click-me" and load in other TRANSLATIONS object named ISLAND_TRANSLATIONS as this identifier is the unique used by the island. Currently, the other "welcome-to-leptos" identifier would be included in the code shipped to the client to hydrate the island.

examples/ssr-hydrate-axum-islands/src/fileserv.rs Outdated Show resolved Hide resolved
examples/ssr-hydrate-axum-islands/README.md Outdated Show resolved Hide resolved
examples/ssr-hydrate-axum-islands/.gitignore Outdated Show resolved Hide resolved
@mondeja
Copy link
Owner

mondeja commented Oct 2, 2024

  • I'm currently reloading the page after changing the language because, without the reload, the changes don't propagate. It seems that the i18n context in LanguageSelector differs from the one in Counter.

Because LanguageSelector is not a children of Counter. Create a parent island to provide the context there and use it in their children.

                                  #[island] Parent     leptos_fluent!

#[island] Child1     move_tr!                                   #[island] Child2     expect_i18n().set_language()

Remember, one context per archipelago or island.

@NCura
Copy link
Contributor Author

NCura commented Oct 3, 2024

I'm not entirely sure I understand the intended workflow for managing multiple translation directories.

Currently, I’ve structured the locales folder at the root of the example, with subfolders for each language, each containing a main.ftl file. In app.rs, I reference this structure both when creating the static TRANSLATIONS array and when initializing the i18n context via the leptos_fluent! macro. And then the same in home.rs.

Right now, I’m using translations in a #[component] on the server as well as in an #[island] that is shipped to the client. To avoid sending unnecessary data to the client, you suggested separating the translation directories.

Would this structure work for that?

- locales
    - core
        - en
        - es
    - server
        - en
        - es
    - island-home
        - en
        - es
    - island-contact
        - en
        - es
    - languages.json
  • The core directory would hold core_locales, containing translations used across multiple places (e.g., in a component and across several "archipelagos", like a common slogan).

  • The server directory would be used for the TRANSLATIONS array in app.rs.

Create a parent island to provide the context there and use it in their children.

  • Following this, I would create a separate folder for each "archipelago." For example, the home module has a parent island that loads its specific folder, and the contact page has another module where its parent island would load its own folder.

Does this align with the correct workflow?

@NCura
Copy link
Contributor Author

NCura commented Oct 3, 2024

The current implementation gives an error in the browser console when trying to switch languages, complaining about I18n context is missing. Is there a problem with the way I created the parent island and added the children islands?

@mondeja
Copy link
Owner

mondeja commented Oct 3, 2024

I've pushed a fix for the example. Does it resolve your questions?

  • The core directory would hold core_locales, containing translations used across multiple places (e.g., in a component and across several "archipelagos", like a common slogan).

This is a misunderstanding about what core_locales does. This fluent-templates' setting allows to share a common Fluent resource across multiple locales, not a locales directory between multiple translations.

Comment on lines 93 to 100
// Children will be executed when the i18n context is ready.
// See https://book.leptos.dev/islands.html#passing-context-between-islands
//
// > That’s why in `HomePage`, I made `let tabs = move ||` a function, and
// > called it like `{tabs()}`: creating the tabs lazily this way meant that
// > the `Tabs` island would already have provided the `selected` context by
// > the time each `Tab` went looking for it.
children()
Copy link
Owner

@mondeja mondeja Oct 3, 2024

Choose a reason for hiding this comment

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

Is there a problem with the way I created the parent island and added the children islands?

This is the answer. See the final note in Passing Context Between Islands:

That’s why in HomePage, I made let tabs = move || a function, and called it like {tabs()}: creating the tabs lazily this way meant that the Tabs island would already have provided the selected context by the time each Tab went looking for it.

Is really tricky and very contraintuitive.

Just realize that when you set a <Component/> in a view!, you're not instantiating an object, you're just creating an AST node that will be rendered who knows when and in what conditions. In this example, seems like we're instantiating <Counter/> and <LanguageSelector/> in the server, but is not the case.

@NCura
Copy link
Contributor Author

NCura commented Oct 4, 2024

Thank you for fixing the example! I've added more content to simulate a common setup for multilanguage sites, where the language selector is often placed in the header, footer, or both. I thus introduced some nested routes.

  • In the header module, I added some navigation links along with the LanguageSelector, as most users would typically place these in the header. The links don’t need to be inside an island, since they can inherit the i18n context from App(). To test behavior, I placed one link before the LanguageSelector and one after. I expected both links to be translated, but only the first one is. The LanguageSelector loads its own context, which I assumed would remain confined to the island, without affecting translations from the #[component] running on the server.

  • The home page demonstrates a case where interactive components are spread out. I'm unsure whether multiple archipelagos are the right approach here or even if a single locale would suffice. The LanguageSelector changes the i18n context, so I have to reuse the i18n! macro with the server locale to translate the h1 tag. However, the p tag, despite having the same key as the h1, remains untranslated. Again, it seems the first Archipelago2 (which loads its own context) is overriding the i18n context for the p tag, even though the p is outside the island.

  • Page 2 serves as an example of how additional pages need to manage translations. If I understand correctly, components running on the server should be able to access the i18n context, but that’s not happening in this case. I’ve had to reuse the i18n! macro again. For islands, each requires its own TRANSLATIONS array.

That’s the current layout of the example, but now I need to make it work as expected. At present, when the language is changed, only the text within the same island as the LanguageSelector updates. The rest of the site retains the previous language. Reloading the page resolves the issue and updates all translations correctly.

Do you have any suggestions on how to ensure that changing the language via the LanguageSelector propagates to the other i18n contexts? Or am I approaching this from the wrong angle?

Comment on lines 97 to 102
let i18n = expect_i18n();
view! {
<header>
<A href="/">{move_tr!("home")}</A>
<LanguageSelector/>
<A href="/page-2">{move_tr!("page-2")}</A>
<A href="/">{move_tr!(i18n, "home")}</A>
<LanguageSelector />
<A href="/page-2">{move_tr!(i18n, "page-2")}</A>
Copy link
Owner

@mondeja mondeja Oct 7, 2024

Choose a reason for hiding this comment

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

  • In the header module, I added some navigation links along with the LanguageSelector, as most users would typically place these in the header. The links don’t need to be inside an island, since they can inherit the i18n context from App(). To test behavior, I placed one link before the LanguageSelector and one after. I expected both links to be translated, but only the first one is. The LanguageSelector loads its own context, which I assumed would remain confined to the island, without affecting translations from the #[component] running on the server.

This is another unintuitive case of provide_context / use_context behaviour. In Leptos v0.6, contexts are herratically spread from child components in a declarative way, but v0.7 "improves" (?) it by executing the child first and spreading the last to their parents. In v0.6, the first string is translated but in v0.7 none of them are translated.

I'm in the team that components should be always declared using <Context/> components to avoid these kinds of strange behaviours. Even I would remove use_context at all in the future from Leptos itself, looks like the root of all evils. In leptos-fluent v0.2 you'll be forced to pass a children prop leptos_fluent! because internally it will use <Context/>.

To specify exactly what i18n context to use, you can pass it as the first value to move_tr! and tr! macros.

@mondeja
Copy link
Owner

mondeja commented Oct 7, 2024

I think that you're not completely aware about how the reactive graph works. Is not the same as the component tree. Consider the next example:

#[component]
fn Foo() -> impl IntoView {
    provide_context::<usize>(0);

    view! {
        <h1>"Foo"</h1>
        {
            let value = expect_context::<usize>();
            view! {
                <p>"Context value before Bar: "{value}</p>
            }
        }
        <Bar/>
        {
            let value = expect_context::<usize>();
            view! {
                <p>"Context value after Bar -> Baz: "{value}</p>
            }
        }
    }
}

#[component]
fn Bar() -> impl IntoView {
    provide_context::<usize>(1);
    view! {
        <h1>"Bar"</h1>
        {
            let value = expect_context::<usize>();
            view! {
                <p>"Context value before Baz: "{value}</p>
            }
        }
        <Baz/>
    }
}

#[component]
fn Baz() -> impl IntoView {
    provide_context::<usize>(2);
    view! {
        <h1>"Baz"</h1>
    }
}

What should it render? Well... it renders this:

<h1>Foo</h1>
<p>Context value before Bar: <!---->0</p>
<h1>Bar</h1>
<p>Context value before Baz: <!---->1</p>
<h1>Baz</h1>
<p>Context value after Bar -&gt; Baz: <!---->2</p>

Because Baz is a sibling of Foo children in the reactive graph. But you think that is just a children of Bar in the component tree and that is outside the scope of Foo children. That doesn't matter for Leptos.

This is not intuitive at a first glance. It is what is happening in your examples. Just pass an explicit i18n context to the tr! macros to avoid confusion.

@mondeja mondeja linked an issue Oct 7, 2024 that may be closed by this pull request
@mondeja mondeja added the docs Improvements or additions to documentation label Oct 7, 2024
@NCura
Copy link
Contributor Author

NCura commented Oct 8, 2024

Thank you for the clarifications, and for specifying the need to pass the i18n context to every move_tr! and tr! macro. This has solved the issue of ensuring all texts are translated correctly. Now, the remaining challenge is getting the translations to update dynamically when the LanguageSelector changes the language.

I'm currently focusing on the header translation:

#[component]
pub fn View() -> impl IntoView {
    let i18n = expect_i18n();
    view! {
        <header>
            <A href="/">{move_tr!(i18n, "home")}</A>
            <LanguageSelector />
            <A href="/page-2">{move_tr!(i18n, "page-2")}</A>
        </header>
    }
}

#[island]
fn LanguageSelector() -> impl IntoView {
    let i18n = i18n!([TRANSLATIONS], "./locales/header");
    view! {
        <div style="display: inline-flex; margin-left: 10px">
            {move_tr!(i18n, "select-language")} ": "
            {move || {
                i18n.languages
                    .iter()
                    .map(|lang| {
                        view! {
                            <div>
                                <input
                                    type="radio"
                                    id=lang
                                    name="language"
                                    value=lang
                                    checked=lang.is_active()
                                    on:click=move |_| i18n.language.set(lang)
                                />
                                <label for=lang>{lang.name}</label>
                            </div>
                        }
                    })
                    .collect::<Vec<_>>()
            }}

        </div>
    }
}

The goal is to update the text inside the <A> tags when the language changes. Since the LanguageSelector and the other components each have their own i18n context, the text does not update automatically.

I’ve attempted several approaches, including the four patterns outlined here. However, none have worked because the LanguageSelector is an island, and the #[island] macro requires props to be serializable to pass them from the server to the client.

I also tried saving the selected language to localStorage like this inside the LanguageSelector:

on:click=move |_| {
    i18n.language.set(lang);
    leptos_fluent::web_sys::window()
        .unwrap()
        .local_storage()
        .unwrap()
        .unwrap()
        .set_item("selected_language", lang.name)
        .unwrap();
}

But I encountered issues retrieving it inside the View component to update the language dynamically.

Do you have any suggestions on how we can achieve this?

@mondeja
Copy link
Owner

mondeja commented Oct 8, 2024

The goal is to update the text inside the <A> tags when the language changes. Since the LanguageSelector and the other components each have their own i18n context, the text does not update automatically.

Why are you using a #[component] for the view? Doesn't that go against islands design? AFAIK, islands are the interactive pieces of the page in an islands website. The translations that are in the rendered in the server can't be updated in the client, you don't even have the translations on the client.

Remember: #[component]s are rendered in the server and #[island]s are hydrated in the client.

@NCura
Copy link
Contributor Author

NCura commented Oct 9, 2024

This is the internal debate I’ve been having: On the one hand, islands are use for interactive pieces, and I would like the translations to react to the language change. On the other hand, they should be as small and specific as possible. However, if I have a fully translated website with little backend code, it seems I would need an archipelago containing almost all of my components just to share an i18n context. At that point, I feel like I'm losing the size reduction benefits of using islands. Am I missing something?

The best compromise I’ve come up with, when using leptos-fluent and islands, is to limit islands to non-translation-related interactivity and keep all translation handling on the server. When the language changes, the page would reload. While this approach loses the benefit of changing the language without a reload, it significantly reduces the size of the WebAssembly (wasm) sent to the client, especially for sites with minimal interactivity and lots of content.

I’ve used the original example to demonstrate the first approach, where we have a single archipelago that shares the i18n context across components. I made some necessary adjustments based on a bug mentioned by gbj in the Leptos repo issue I raised:

...
2. Using an island inside the view that you return from an island
Either is fine, although 2. is (IIRC) bugged in 0.6 and fixed in 0.7

So I had to refactor the code:

#[island]
pub fn HeaderView() -> impl IntoView {
    view! {
        <header>
            <a href="/">{move_tr!("home")}</a>
            <a href="/page-2">{move_tr!("page-2")}</a>
            <LanguageSelector/>
        </header>
    }
}

into:

#[component]
pub fn HeaderView() -> impl IntoView {
    view! {
        <header>
            <HeaderLinks/>
            <LanguageSelector/>
        </header>
    }
}

#[island]
fn HeaderLinks() -> impl IntoView {
    view! {
            <a href="/">{move_tr!("home")}</a>
            <a href="/page-2">{move_tr!("page-2")}</a>
    }
}

I did similar changes in the home and page_2 modules. After running cargo leptos build --release, the resulting wasm file size was 382k. For reference, I ran the same project without islands, and the wasm size increased to 561k.

I also added a second example, keeping the translations on the server. The downside is that the page must reload when changing the language, as mentioned before. The upside is that the wasm size is reduced to 282k, and this size won’t grow even as more content is added to the site. In contrast, the first example (382k) will continue to increase in size with additional content.

Would it make sense to include both examples (or perhaps one example but with explanations for both approaches)? This way, users of leptos-fluent and the islands feature can choose the path that best fits their project—whether they prefer client-side translations with a larger wasm or server-side translations with page reloads.

@mondeja
Copy link
Owner

mondeja commented Oct 9, 2024

Would it make sense to include both examples (or perhaps one example but with explanations for both approaches)? This way, users of leptos-fluent and the islands feature can choose the path that best fits their project—whether they prefer client-side translations with a larger wasm or server-side translations with page reloads.

I think that should be added just one example for islands using the second approach with page reloads which is the one that has more sense with islands because exploits better the benefit of file size, the main goal of islands design. IMHO, including both examples would add unnecessary noise for users who consult them.

To provide more context, I would suggest you include an explanation of this lack of interactivity issue in the README and/or in comments within the code, probably where you trigger the site reload.

@NCura
Copy link
Contributor Author

NCura commented Oct 9, 2024

That sounds great to me! I’ll implement this tomorrow. As an example, I’ve already updated one of my client’s websites from the first approach to the second in the development environment. The result was a significant reduction in the WebAssembly size, going from just over 1000KB down to less than 400KB. Given this improvement, the trade-off of a page reload is definitely worth it.

@NCura
Copy link
Contributor Author

NCura commented Oct 10, 2024

Additional Changes to the Final Example

  • Added both a mobile and a large menu to demonstrate the archipelago pattern, with some translated text inside islands, and passing context between islands.
  • Simplified the home module, as we already have enough usage examples in the header.
  • Included a README.md file. Feel free to update or improve any sections you think could be clearer.
  • Added a brief comment in the LanguageSelector, explaining the page reload and referencing the README.md for further details.

Request for Clarification

In the i18n context setup within both App and LanguageSelector, I'm unsure about which arguments should be used on the server versus the client. You mentioned in a previous issue:

Well, note that some of the arguments only make sense for the server and others for the client.

Could you help clean up the arguments in the leptos_fluent! macro, ensuring that the one in App only contains server-side arguments, and the one in LanguageSelector has the client-side ones?

Follow-up Questions

  • Since we're on the server, could we use tr instead of move_tr, or is there still a reason to use the reactive version?
  • In the i18n context created within LanguageSelector, shouldn't we use a translations array containing only the necessary translations, rather than pulling in all server-side translations? However, if we limit the translations, it seems that anything after the header won't be translated, as only the client-side translations will be available.
  • I encountered a small issue: I added the LanguageSelector twice—once in the normal menu and once in the mobile menu. On larger screen sizes, the input for the selected language in the normal menu has the checked attribute, but it doesn't display as checked, while the one in the mobile menu does.

@mondeja
Copy link
Owner

mondeja commented Oct 11, 2024

Could you help clean up the arguments in the leptos_fluent! macro, ensuring that the one in App only contains server-side arguments, and the one in LanguageSelector has the client-side ones?

Cleaned.

  • Since we're on the server, could we use tr instead of move_tr, or is there still a reason to use the reactive version?

Yes, we should use tr! if no interactivity is expected.

  • I encountered a small issue: I added the LanguageSelector twice—once in the normal menu and once in the mobile menu. On larger screen sizes, the input for the selected language in the normal menu has the checked attribute, but it doesn't display as checked, while the one in the mobile menu does.

Because all <input>s have the same "language" name. Fixed now.

  • In the i18n context created within LanguageSelector, shouldn't we use a translations array containing only the necessary translations, rather than pulling in all server-side translations? However, if we limit the translations, it seems that anything after the header won't be translated, as only the client-side translations will be available.

That's the question with islands and I think that leptos-fluent is not enough featured to allow this for now. However, there is a really dirty workaround that we can apply now to the example.

The most simple solution when you don't need interactivity would be to pass translations as island arguments. Something like:

#[component]
fn MyServerComponent() -> impl IntoView {
    view! {
        <MyIsland translated_message=tr!("foo") />
    }
}

#[island]
fn MyIsland(translated_message: String) -> impl IntoView {
    view! {
        <p>{translated_message}</p>
    }
}

In the case of this example is not enough. Even if we pass a HashMap with a subset of the translations to the top-level island and provide it as a context, you still need the languages to build the language selector.

Currently leptos_fluent! does not allow to set translations: [] as an argument to leptos_fluent!. Opened #250 to allow this. Currently the limitation can be bypassed by including a locales folder with empty translations.

That would solve the problem for this example by using something like:

#[component]
fn MyServerComponent() -> impl IntoView {
    view! {
        <MyArchipelago translations=HashMap::from([
            ("foo", tr!("foo")),
            ("bar", tr!("bar")),
        ]) />
    }
}

#[island]
fn MyArchipelago(translations: HashMap<String, String>) -> impl IntoView {
    provide_context(translations)

    view! {
        <MyIsland />
    }
}

#[island]
fn MyIsland() -> impl IntoView {
    let tr = expect_context::<HashMap<String, String>>();
    view! {
        <p>{tr.get("foo").unwrap()}</p>
        <p>{tr.get("bar").unwrap()}</p>
    }
}

And though it works, the syntax looks weird.

Other current workaround is to duplicate the repeated keys in a separated locales folder, but that is not convenient.

Another possible solution would be to implement some kind of option to leptos_fluent! to just include a subset of the translations by their identifiers. Maybe a land: ["foo", "bar"]. I'm open to ideas. Let me know what you prefer as you seem to be tackling a real project with islands.

@NCura
Copy link
Contributor Author

NCura commented Oct 11, 2024

I saw the PR allowing the creation of an i18n context with an empty array — awesome!

In this case, given that some translations are required on the client for the header, I added a CLIENT_TRANSLATIONS array and the corresponding locale folder. After that, I checked the Network tab in the browser: the wasm file is 4.4MB (since it's not in release mode). I then added a large test translation to the server/en.md file, and the wasm size remained 4.4MB. When I added that translation to client/en.md to test, the wasm size increased to 4.6MB, so it looks like the server translations are correctly excluded from the client-side wasm.

However, the issue now is that the texts in the home and page_2 modules aren't being translated, because the i18n context is using CLIENT_TRANSLATIONS, while their translations are in SERVER_TRANSLATIONS.

To fix this, I called the i18n context function from App in header::View, and now everything is being translated properly. The wasm size is still 4.4MB.

#[component]
pub fn View() -> impl IntoView {
    view! {
        <header>
            <Archipelago>
                <LargeMenu />
                <MobileMenu />
            </Archipelago>
            {super::provide_i18n_context();}
        </header>
    }
}

If you think that there is a better place to call the context again, feel free to tell me. Using it in header::View makes it explicit that since we’re using an Archipelago that loads its own i18n context, we need to reset the context by providing the server one again.

What do you think of this solution? It seems to be working as expected and prevents the server-side translations from being sent to the client, but I'd appreciate your thoughts in case I’m overlooking something.

P.D.: I've kept the test translation text inside the server/en.md in case you want to use it inside the client/en.md to check the file size, but we can remove it whenever you want.

@mondeja
Copy link
Owner

mondeja commented Oct 12, 2024

To fix this, I called the i18n context function from App in header::View, and now everything is being translated properly.

If you call leptos_fluent! a second time, all the code that generates the macro will be created again, probably generating bugs because side effects could become desynchronized. The proper solution is to reprovide the context. I've fixed it.

@NCura
Copy link
Contributor Author

NCura commented Oct 12, 2024

Okay, got it!
Do you want to keep the test translation inside server/en?
Everything else seems fine to me.

@NCura NCura marked this pull request as ready for review October 12, 2024 16:39
@mondeja
Copy link
Owner

mondeja commented Oct 12, 2024

Awesome. Thank you for the great work!

@mondeja mondeja merged commit eed544c into mondeja:master Oct 12, 2024
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with leptos_fluent and Islands feature in Leptos
2 participants