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

<!--#--> in web title when translation string is used #105

Closed
adundovi opened this issue Jan 4, 2022 · 20 comments
Closed

<!--#--> in web title when translation string is used #105

adundovi opened this issue Jan 4, 2022 · 20 comments
Assignees
Labels
A-i18n Area: internationalization A-templates Area: templates C-bug Category: bug P-high Priority: high

Comments

@adundovi
Copy link
Contributor

adundovi commented Jan 4, 2022

Describe the bug
Recently, a regression is introduced when displaying a head title. It occurs when the translation marco t!() is used within the head title tag of a page and instead of displaying only a translated string, comment tags are inserted, thus, the final translated string is displayed in a browser as <!--#-->translated string<!--/-->.

I don't know is it due to sycamore or perseus but since it occurs when t! macro is used, I report it here. When an ordinary string variable is used, everything is fine.

To Reproduce

One can use the i18n example, and add the head function with the title:

pub fn get_template<G: Html>() -> Template<G> {
    Template::new("about")
        .template(about_page)
        .head(head)
}

#[perseus::head]
pub fn head() -> View<SsrNode> {
    view! {
         title { (t!("about")) }
    }
}

Expected behavior
A translated string is displayed without comment tags.

Environment (please complete the following information):

  • Perseus Version: 0.3.1
  • Sycamore Version: 0.7
  • OS: GNU/Linux
  • Browser: Firefox
  • Browser Version: 95
@arctic-hen7
Copy link
Member

Does this still occur if you remove the hydrate feature flag?

@adundovi
Copy link
Contributor Author

adundovi commented Jan 4, 2022

Both, with hydrate and without hydrate, I reproduce the problem and I performed perseus clean before testing.

@arctic-hen7
Copy link
Member

Hmm okay. I'll get on this!

@arctic-hen7 arctic-hen7 self-assigned this Jan 4, 2022
@arctic-hen7 arctic-hen7 added A-i18n Area: internationalization A-templates Area: templates C-bug Category: bug P-high Priority: high labels Jan 4, 2022
@arctic-hen7
Copy link
Member

Even after running perseus clean, that reproduction in the i18n example (also removing the <title> element from index.html), doesn't produce this bug for me on Firefox 96.0b10 running on Ubuntu. Maybe there's something else going on here, could you post a self-contained MRE?

@arctic-hen7
Copy link
Member

arctic-hen7 commented Jan 5, 2022

I have two hypotheses for this bug though:

  1. It's some manifestation of Fluent's default behavior of encasing translated strings in invisible marker elements (they have something to do with text direction from memory).
  2. It's something to do with empty Sycamore views (a View::empty() renders as <!---->), but I'm not sure where the hashes would be coming from then.

@adundovi
Copy link
Contributor Author

adundovi commented Jan 5, 2022

Thanks @arctic-hen7 for looking into this. I'll try to give you a MRE soon.

@lukechu10
Copy link
Contributor

lukechu10 commented Jan 6, 2022

I believe this is because of sycamore ssr codegen. I'll take a look in this as well

EDIT: This is most likely due to the way dynamic values are handled when rendering with SsrNode. The extra comment markers are needed to correctly hydrate the dynamic value but it seems like it is somehow getting into the title text.

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

Even after running perseus clean, that reproduction in the i18n example (also removing the <title> element from index.html), doesn't produce this bug for me on Firefox 96.0b10 running on Ubuntu. Maybe there's something else going on here, could you post a self-contained MRE?

OK, what was missing is an additional string next to the translated one, such as:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    view! {
         title { (t!("about")) " > Test" }
    }
}

This reproduces the problem in a clean i18n example.

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

I've tested without the translation string:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    let foo = "Foo".to_string();
    view! {
         title { (foo) " | Bar" }
    }
}

image

and with it:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    view! {
         title { (t!("about")) " | Bar" }
    }
}

image

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

Hmm, on the other hand - the following works without the issue:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    let foo = t!("about");
    view! {
         title { (foo) " | Bar" }
    }
}

Maybe this depends on how view! macro is expanded?

@arctic-hen7
Copy link
Member

Ah I see. @lukechu10 this should be fixable by using SSR while explicitly telling Sycamore that it doesn't need to prepare for hydration (the head will never be hydrated). Is that possible?

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

Just for the record, this is the minimal issue-triggering case:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    view! {
         title { ("Foo".to_string()) " | Bar" }
    }
}

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

Ah I see. @lukechu10 this should be fixable by using SSR while explicitly telling Sycamore that it doesn't need to prepare for hydration (the head will never be hydrated). Is that possible?

Is this related to hydration? The issue persists even without the hydration feature in Cargo.toml.

@lukechu10
Copy link
Contributor

Hmm, on the other hand - the following works without the issue:

#[perseus::head]
pub fn head() -> View<SsrNode> {
    let foo = t!("about");
    view! {
         title { (foo) " | Bar" }
    }
}

Maybe this depends on how view! macro is expanded?

The reason this works is because there is a simple heuristic in view! to deduce that foo can never be a dynamic value.

@arctic-hen7
Copy link
Member

Is this related to hydration? The issue persists even without the hydration feature in Cargo.toml.

No, I read through that chain a little hastily and misunderstood how view! was handling this. Given this doesn't seem to be a Perseus issue, shall we transfer this to Sycamore?

@adundovi
Copy link
Contributor Author

adundovi commented Jan 6, 2022

No, I read through that chain a little hastily and misunderstood how view! was handling this. Given this doesn't seem to be a Perseus issue, shall we transfer this to Sycamore?

Of course.

@arctic-hen7
Copy link
Member

By the same token, I'm also thinking that the t! macro might be able to somehow identify itself to Sycamore as unchanging. I'll play with this a bit...

@arctic-hen7
Copy link
Member

It doesn't look like Perseus can tell Sycamore that the t! macro will never be reactive, but using format! does work, so I'd advise using that in the general case.

Given that this seems to be solely a Sycamore issue, I'll close this.

@lukechu10
Copy link
Contributor

It doesn't look like Perseus can tell Sycamore that the t! macro will never be reactive, but using format! does work, so I'd advise using that in the general case.

format! works? Normally, format! and t! are treated the exact same way...

@arctic-hen7
Copy link
Member

No, putting everything inside a format! block instead of putting things next to each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-i18n Area: internationalization A-templates Area: templates C-bug Category: bug P-high Priority: high
Projects
None yet
Development

No branches or pull requests

3 participants