search

nanojsx / nano

🎯 SSR first, lightweight 1kB JSX library.
http://nanojsx.io
MIT License
1.45k stars 38 forks source link

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

Closed aiotter closed 2 years ago

aiotter commented 2 years ago

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.

jrson83 commented 2 years ago

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 , <meta> tags, and other <head> elements.</p> </blockquote> <p>Same goes for Nano JSX <code><Helmet></code> as far as I understand, it does not like React Helmet yet</p> <blockquote> <p>Support attributes for html</p> </blockquote> <p>If you check the <a href="https://github.com/nanojsx/template/blob/master/src/server/makeHtml.tsx">template makeHTML</a>, you see <code>${head.join('\n')}</code> is just additionally added/rendered to the html markup in side the <strong><code><head></code></strong>. So <code><html lang="en"></code> is out of scope for Helmet.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/yandeu"><img src="https://avatars.githubusercontent.com/u/20306025?v=4" />yandeu</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>As of now <code>Nano.Helmet.SSR</code> returns <code>{ body, head, footer }</code>.</p> <p>I guess it should return <code>{ body, head, footer, html }</code>, where html would be <code><html prefix="og: https://ogp.me/ns#"></code> (open tag only).</p> <p>@aiotter What do you think?</p> <hr /> <p>Edit: Or better, <code>html</code> should be an object of attributes like:</p> <pre><code class="language-js">{ prefix: 'og: https://ogp.me/ns#', lang: 'en' }</code></pre> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/aiotter"><img src="https://avatars.githubusercontent.com/u/37664775?v=4" />aiotter</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>@yandeu <code>{ prefix: 'og: https://ogp.me/ns#', lang: 'en' }</code> seems nice! How about implementing <code>toString()</code> method to this object, which returns <code>prefix="og: https://ogp.me/ns#'" lang="en"</code>? This will make people easier to embed the attributes into their HTML string. React-helmet has the similar one, <code>helmet.htmlAttributes.toString()</code>. FYR: <a href="https://github.com/nfl/react-helmet#as-string-output">https://github.com/nfl/react-helmet#as-string-output</a>.</p> <p>I afraid <code>html</code> attribute of the returned value of <code>Nano.Helmet.SSR()</code> becoming different from that of <code>body</code> attribute then. They can also set the attribute of <code>body</code> tag with <code>Nano.Helmet</code>, and it has the same problem. How should it be?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/aiotter"><img src="https://avatars.githubusercontent.com/u/37664775?v=4" />aiotter</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>btw, I feel it better to have a option to return <code>body</code> and <code>head</code> as components instead of string. I'm coding like following, but I feel it a bit too roundabout.</p> <pre><code class="language-tsx"> 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> ));</code></pre> <p>react-helmet has nice way to accomplish this, say:</p> <pre><code class="language-tsx">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> ); }</code></pre> <p>If you feel this possible, I'll create a new issue for it.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/yandeu"><img src="https://avatars.githubusercontent.com/u/20306025?v=4" />yandeu</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>The body returned from <code>Nano.Helmet.SSR()</code> does NOT include the body attributes.</p> <p>This looks right:</p> <pre><code class="language-js">const { body, head, footer, attributes: { html, body } } = Helmet.SSR(app)</code></pre> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/yandeu"><img src="https://avatars.githubusercontent.com/u/20306025?v=4" />yandeu</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>What about this: <em><a href="https://github.com/nanojsx/nano/tree/improve-helmet">improve-helmet branch</a>; test with <code>npm run ssr</code></em></p> <pre><code class="language-jsx">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"</code></pre> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/aiotter"><img src="https://avatars.githubusercontent.com/u/37664775?v=4" />aiotter</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>Looks really nice! That makes us code like this:</p> <pre><code class="language-tsx">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> );</code></pre> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/yandeu"><img src="https://avatars.githubusercontent.com/u/20306025?v=4" />yandeu</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>Or even simpler:</p> <pre><code class="language-js">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>` }</code></pre> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/aiotter"><img src="https://avatars.githubusercontent.com/u/37664775?v=4" />aiotter</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>@yandeu Thank you for implementing this! I cannot wait to test it on my deno project; can you run denoify to update the <code>deno_lib</code>? So that I can test it before new release!</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/yandeu"><img src="https://avatars.githubusercontent.com/u/20306025?v=4" />yandeu</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>I just released v0.0.28</p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>