<html> tag in <Helmet> is not handled properly on SSR #86

aiotter · 2022-01-03T05:24:51Z

Describe the bug

/** @jsx Nano.h */
/** @jsxFrag Nano.Fragment */
/// <reference no-default-lib="true" />
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
/// <reference lib="deno.ns" />
/// <reference lib="deno.unstable" />

import * as Nano from "https://deno.land/x/nano_jsx@v0.0.26/mod.ts";

const App = () => (
  <>
    <Nano.Helmet>
      <html prefix="og: https://ogp.me/ns#" />
      <meta charset="utf-8" />
      <title>Title</title>
      <meta property="og:type" content="website" />
    </Nano.Helmet>
    <h1>Hello, world!</h1>
  </>
);
const { body, head } = Nano.Helmet.SSR(Nano.renderSSR(() => <App />));
console.log(head);  // ['<html prefix="og: https://ogp.me/ns#"></html><meta charset="utf-8" /><title>Title</title><meta conte...']
console.log(body);  // <h1>Hello, world!</h1>

Nano.Helmet#didMount handles <html> tag, so it won't work for SSR, resulting to emit incorrect <html> tag in head property of the returning object of Nano.Helmet.SSR().

To prevent this, Nano JSX should at least delete <html> tag or raise an error on Nano.Helmet.SSR(). It would be better if it returns htmlAttributes and bodyAttributes like react-helmet does.

The text was updated successfully, but these errors were encountered:

jrson83 · 2022-01-03T07:17:08Z

This is just a notice since your idea might be good and correct me if I'm wrong.

This is a notice from InertiaJS Title & meta:

Since JavaScript apps are rendered within the document , they are unable to render markup to the document , as it's outside of their scope. To help with this, Inertia ships with an component, which can be used to set the page <title>, tags, and other elements.

Same goes for Nano JSX <Helmet> as far as I understand, it does not like React Helmet yet

Support attributes for html

If you check the template makeHTML, you see ${head.join('\n')} is just additionally added/rendered to the html markup in side the <head>. So <html lang="en"> is out of scope for Helmet.

yandeu · 2022-01-03T09:02:34Z

As of now Nano.Helmet.SSR returns { body, head, footer }.

I guess it should return { body, head, footer, html }, where html would be <html prefix="og: https://ogp.me/ns#"> (open tag only).

@aiotter What do you think?

Edit:
Or better, html should be an object of attributes like:

{ prefix: 'og: https://ogp.me/ns#', lang: 'en' }

aiotter · 2022-01-03T09:47:34Z

@yandeu { prefix: 'og: https://ogp.me/ns#', lang: 'en' } seems nice!
How about implementing toString() method to this object, which returns prefix="og: https://ogp.me/ns#'" lang="en"? This will make people easier to embed the attributes into their HTML string. React-helmet has the similar one, helmet.htmlAttributes.toString().
FYR: https://github.com/nfl/react-helmet#as-string-output.

I afraid html attribute of the returned value of Nano.Helmet.SSR() becoming different from that of body attribute then. They can also set the attribute of body tag with Nano.Helmet, and it has the same problem. How should it be?

aiotter · 2022-01-03T10:07:04Z

btw, I feel it better to have a option to return body and head as components instead of string.
I'm coding like following, but I feel it a bit too roundabout.

  const app = Nano.renderSSR(() => <App />);
  const { body, head } = Nano.Helmet.SSR(app);
  const html = Nano.renderSSR(() => (
    <html prefix="og: https://ogp.me/ns#">
      <head innerHTML={{ __dangerousHtml: head.join("\n") }} />
      <body innerHTML={{ __dangerousHtml: body }} />
    </html>
  ));

react-helmet has nice way to accomplish this, say:

function HTML () {
    const htmlAttrs = helmet.htmlAttributes.toComponent();
    const bodyAttrs = helmet.bodyAttributes.toComponent();

    return (
        <html {...htmlAttrs}>
            <head>
                {helmet.title.toComponent()}
                {helmet.meta.toComponent()}
                {helmet.link.toComponent()}
            </head>
            <body {...bodyAttrs}>
                <div id="content">
                    // React stuff here
                </div>
            </body>
        </html>
    );
}

If you feel this possible, I'll create a new issue for it.

yandeu · 2022-01-03T20:33:58Z

The body returned from Nano.Helmet.SSR() does NOT include the body attributes.

This looks right:

const { body, head, footer, attributes: { html, body } } = Helmet.SSR(app)

yandeu · 2022-01-03T21:30:13Z

What about this:
improve-helmet branch; test with npm run ssr

class App extends Component {
  render() {
    return (
      <div>
        <h1>Nano JSX App</h1>
        <Helmet>
          <html lang="en" amp />

          <body class="root" />
          <body class="main" id="id" />

          <title>My Title</title>

          <meta name="description" content="Nano-JSX application" />
        </Helmet>
      </div>
    )
  }
}

const app = renderSSR(<App />)
const { attributes, head } = Helmet.SSR(app)

console.log(head) // <title>My Title</title><meta content="Nano-JSX application" name="description" />
console.log(attributes.body) // [Map] { 'class' => 'main', 'id' => 'id' }
console.log(attributes.html) // [Map] { 'lang' => 'en', 'amp' => 'true' }
console.log(attributes.body.toString()) // class="main" id="id"
console.log(attributes.html.toString()) // lang="en" amp="true"

aiotter · 2022-01-04T03:12:57Z

Looks really nice! That makes us code like this:

const app = renderSSR(<App />)
const { attributes, body, head } = Helmet.SSR(app)

const Html = () => (
  <html {...Object.fromEntries(attributes.html)}>
    <head innerHTML={{ __dangerousHtml: head.join('\n') }} />
    <body {...Object.fromEntries(attributes.body)} innerHTML={{ __dangerousHtml: body }} />
  </html>
);

yandeu · 2022-01-04T11:03:28Z

Or even simpler:

const app = renderSSR(<App />)
const { body, head, footer, attributes } = Helmet.SSR(app)

const Html = () => {
  return `
    <!DOCTYPE html>
    <html ${attributes.html.toString()}>
      <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        ${head.join('\n')}
      </head>
      <body ${attributes.body.toString()}>>
        ${body}
        ${footer.join('\n')}
      </body>
    </html>`
}

aiotter · 2022-01-13T06:24:17Z

@yandeu Thank you for implementing this!
I cannot wait to test it on my deno project; can you run denoify to update the deno_lib? So that I can test it before new release!

yandeu · 2022-01-13T21:34:10Z

I just released v0.0.28

yandeu closed this as completed in 2a4b738 Jan 9, 2022

yandeu mentioned this issue Jan 9, 2022

Docs: Write docs for #86 #91

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

<html> tag in <Helmet> is not handled properly on SSR #86

<html> tag in <Helmet> is not handled properly on SSR #86

aiotter commented Jan 3, 2022 •

edited

jrson83 commented Jan 3, 2022

yandeu commented Jan 3, 2022 •

edited

aiotter commented Jan 3, 2022 •

edited

aiotter commented Jan 3, 2022

yandeu commented Jan 3, 2022

yandeu commented Jan 3, 2022

aiotter commented Jan 4, 2022 •

edited

yandeu commented Jan 4, 2022

aiotter commented Jan 13, 2022

yandeu commented Jan 13, 2022

<html> tag in <Helmet> is not handled properly on SSR #86

<html> tag in <Helmet> is not handled properly on SSR #86

Comments

aiotter commented Jan 3, 2022 • edited

jrson83 commented Jan 3, 2022

yandeu commented Jan 3, 2022 • edited

aiotter commented Jan 3, 2022 • edited

aiotter commented Jan 3, 2022

yandeu commented Jan 3, 2022

yandeu commented Jan 3, 2022

aiotter commented Jan 4, 2022 • edited

yandeu commented Jan 4, 2022

aiotter commented Jan 13, 2022

yandeu commented Jan 13, 2022

aiotter commented Jan 3, 2022 •

edited

yandeu commented Jan 3, 2022 •

edited

aiotter commented Jan 3, 2022 •

edited

aiotter commented Jan 4, 2022 •

edited