From 0c0fae4487bfa944fb91757f1279a49b631a2012 Mon Sep 17 00:00:00 2001 From: Maxime Thirouin Date: Tue, 15 Nov 2016 22:37:20 +0100 Subject: [PATCH] Refactor the way html files are wrapped. Now it's way more flexible. --- flow/interfaces/node-modules/react-helmet.js | 38 ++++++- package.json | 1 + src/_utils/html-metas/__tests__/index.js | 38 ------- src/_utils/html-metas/index.js | 25 ----- .../__tests__/__snapshots__/index.js.snap | 31 ++++++ src/components/Html/__tests__/index.js | 52 ++------- src/components/Html/index.js | 78 ++++++++++---- .../results/destination/index.html.js | 11 +- .../destination/test-url/index.html.js | 11 +- .../results/destination/test/index.html.js | 11 +- src/static/__tests__/url-as-html.js | 25 +++-- src/static/url-as-html.js | 102 ++++++++---------- 12 files changed, 207 insertions(+), 216 deletions(-) delete mode 100644 src/_utils/html-metas/__tests__/index.js delete mode 100644 src/_utils/html-metas/index.js create mode 100644 src/components/Html/__tests__/__snapshots__/index.js.snap diff --git a/flow/interfaces/node-modules/react-helmet.js b/flow/interfaces/node-modules/react-helmet.js index 7936dc2ee..e5cecfa3d 100644 --- a/flow/interfaces/node-modules/react-helmet.js +++ b/flow/interfaces/node-modules/react-helmet.js @@ -1,9 +1,37 @@ -declare module "react-helmet" { - declare function rewind(): void; +/* eslint-disable max-len */ +// https://github.com/flowtype/flow-typed/tree/master/definitions/npm/react-helmet_v3.x.x - declare interface ReactHelmet { - rewind: rewind +declare module "react-helmet" { + declare type Props = { + htmlAttributes?: Object, + title?: string, + defaultTitle?: string, + titleTemplate?: string, + base?: Object, + meta?: Array, + link?: Array, + script?: Array, + noscript?: Array, + style?: Array, + onChangeClientState?: (newState: Object, addedTags: Object, removeTags: Object) => void | mixed, + }; + declare interface HeadAttribute { + toString(): string; + toComponent(): React$Element<*>; + } + declare interface Head { + htmlAttributes: HeadAttribute; + title: HeadAttribute; + base: HeadAttribute; + meta: HeadAttribute; + link: HeadAttribute; + script: HeadAttribute; + style: HeadAttribute; } - declare var exports: ReactHelmet + declare class Helmet extends React$Component { + static rewind(): Head; + props: Props; + } + declare var exports: typeof Helmet; } diff --git a/package.json b/package.json index b79db2f03..897095382 100644 --- a/package.json +++ b/package.json @@ -123,6 +123,7 @@ "react-helmet": "^3.0.0", "react-redux": "^4.0.0", "react-router": "^2.3.0", + "react-test-renderer": "^15.3.2", "redux": "^3.0.0", "stylelint": "^7.2.0", "stylelint-config-standard": "^13.0.0", diff --git a/src/_utils/html-metas/__tests__/index.js b/src/_utils/html-metas/__tests__/index.js deleted file mode 100644 index 4004ccce2..000000000 --- a/src/_utils/html-metas/__tests__/index.js +++ /dev/null @@ -1,38 +0,0 @@ -import url from "url" - -import test from "jest-ava-api" - -import htmlMetas, { defaultMetas } from "../" - -test("html default metas", (t) => { - t.deepEqual( - htmlMetas(), - defaultMetas - ) -}) - -test("html static metas", (t) => { - t.deepEqual( - htmlMetas({ - baseUrl: url.parse("http://domain.ext/"), - css: [ "phenomic.browser.css" ], - }), - [ - ...defaultMetas, - "", - ] - ) -}) - -test("html static metas with path", (t) => { - t.deepEqual( - htmlMetas({ - baseUrl: url.parse("http://domain.ext/basep/"), - css: [ "phenomic.browser.css" ], - }), - [ - ...defaultMetas, - "", - ] - ) -}) diff --git a/src/_utils/html-metas/index.js b/src/_utils/html-metas/index.js deleted file mode 100644 index ec17f8612..000000000 --- a/src/_utils/html-metas/index.js +++ /dev/null @@ -1,25 +0,0 @@ -// @flow -import pathToUri from "../path-to-uri" - -export const defaultMetas = [ - "", - "", - "", -] - -export default function( - { baseUrl, css }: {baseUrl: {pathname: string}, css: Array } = {} -): Array { - const metas = [ ...defaultMetas ] - - if (css && Array.isArray(css)) { - css.forEach(fileName => { - const path = pathToUri(baseUrl.pathname, fileName) - metas.push( - `` - ) - }) - } - - return metas -} diff --git a/src/components/Html/__tests__/__snapshots__/index.js.snap b/src/components/Html/__tests__/__snapshots__/index.js.snap new file mode 100644 index 000000000..9b811c66a --- /dev/null +++ b/src/components/Html/__tests__/__snapshots__/index.js.snap @@ -0,0 +1,31 @@ +exports[`test should render Html componnent 1`] = ` + + + + <meta + charSet="utf-8" + data-react-helmet={true} /> + <meta + content="IE=edge" + data-react-helmet={true} + httpEquiv="X-UA-Compatible" /> + <meta + content="width=device-width, initial-scale=1" + data-react-helmet={true} + name="viewport" /> + <link + data-react-helmet={true} + href="test.css" + rel="stylesheet" /> + </head> + <body> + <div /> + <script /> + <script + data-react-helmet={true} + src="test.css" /> + </body> +</html> +`; diff --git a/src/components/Html/__tests__/index.js b/src/components/Html/__tests__/index.js index 1d6102169..8d50441b9 100644 --- a/src/components/Html/__tests__/index.js +++ b/src/components/Html/__tests__/index.js @@ -1,50 +1,18 @@ -import test from "jest-ava-api" import React from "react" import { createRenderer } from "react-addons-test-utils" -import expect from "expect" -import expectJSX from "expect-jsx" import Html from "../index.js" -expect.extend(expectJSX) - -const head = "head" -const body = "body" -const script = "script" -const htmlProps = { lang: "en" } - -const renderer = (...args) => { - const render = createRenderer() - render.render(...args) - return render.getRenderOutput() -} - test("should render Html componnent", () => { - expect( - renderer( - <Html - htmlProps={ htmlProps } - head={ head } - body={ body } - script={ script } - config={{ clientScripts: true }} - > - <p>{ "foo" }</p> - </Html> - ) - ).toEqualJSX( - <html - lang="en" - > - <head dangerouslySetInnerHTML={{ __html: head }} /> - <body> - <div - dangerouslySetInnerHTML={{ __html: body }} - id="phenomic" - /> - <script dangerouslySetInnerHTML={{ __html: script }} /> - <p>{ "foo" }</p> - </body> - </html> + const renderer = createRenderer() + renderer.render( + /* eslint-disable react/jsx-no-bind */ + <Html + css={ [ "test.css" ] } + js={ [ "test.css" ] } + renderBody={ () => <div /> } + renderScript={ () => <script /> } + /> ) + expect(renderer.getRenderOutput()).toMatchSnapshot() }) diff --git a/src/components/Html/index.js b/src/components/Html/index.js index dc3425314..db209f42b 100644 --- a/src/components/Html/index.js +++ b/src/components/Html/index.js @@ -1,31 +1,65 @@ // @flow import React from "react" +import { renderToString } from "react-dom/server" +import Helmet from "react-helmet" type Props = { - htmlProps: Object, - head: string, - body: string, - script: string, - config: PhenomicStaticConfig, - children?: any, + css: Array<string>, + js: Array<string>, + renderBody: () => React$Element<any>, + renderScript: () => React$Element<any>, } -const Html = (props: Props) => ( - <html { ...props.htmlProps }> - <head dangerouslySetInnerHTML={{ __html: props.head }} /> - <body> - <div - id="phenomic" - dangerouslySetInnerHTML={{ __html: props.body }} - /> - { - props.config.clientScripts && - <script dangerouslySetInnerHTML={{ __html: props.script }} /> - } - { props.config.clientScripts && props.children } - </body> - </html> -) +const defaultHtmlAttributes = { + "lang": "en", +} +const defaultMeta = [ + // <meta charset="utf-8" /> + { "charset": "utf-8" }, + // <meta http-equiv="X-UA-Compatible" content="IE=edge" /> + { "http-equiv": "X-UA-Compatible", "content": "IE=edge" }, + // <meta name="viewport" content="width=device-width, initial-scale=1" /> + { "name": "viewport", "content": "width=device-width, initial-scale=1" }, +] + +const Html = (props: Props) => { + // Inject default html metas before + // Those need to be rendered somehow otherwise Helmet won't consider those + renderToString( + <Helmet + htmlAttributes={ defaultHtmlAttributes } + meta={ defaultMeta } + link={ [ + ...props.css.map((file) => ({ rel: "stylesheet", href: file })), + ] } + script={ [ + ...props.js.map((file) => ({ src: file })), + ] } + /> + ) + + // render body + const body = props.renderBody() + // rewind html metas + const head = Helmet.rewind() + + // <!doctype html> is automatically prepended + return ( + <html { ...head.htmlAttributes.toComponent() }> + <head> + { head.base.toComponent() } + { head.title.toComponent() } + { head.meta.toComponent() } + { head.link.toComponent() } + </head> + <body> + { body } + { props.renderScript() } + { head.script.toComponent() } + </body> + </html> + ) +} export default Html diff --git a/src/static/__tests__/results/destination/index.html.js b/src/static/__tests__/results/destination/index.html.js index 1f4b63c05..2d0aae13c 100644 --- a/src/static/__tests__/results/destination/index.html.js +++ b/src/static/__tests__/results/destination/index.html.js @@ -1,15 +1,14 @@ /* eslint-disable max-len */ -import htmlMetas from "../../../../_utils/html-metas" - export default () => ( `<!doctype html> <html lang="en"> <head> - ${ htmlMetas({ - baseUrl: { pathname: "/" }, - css: [ "test.css" ] }).join("\n ") } <title data-react-helmet="true"> + + + + @@ -32,7 +31,7 @@ export default () => ( } } - + ` diff --git a/src/static/__tests__/results/destination/test-url/index.html.js b/src/static/__tests__/results/destination/test-url/index.html.js index 2869531c5..bee35a7cc 100644 --- a/src/static/__tests__/results/destination/test-url/index.html.js +++ b/src/static/__tests__/results/destination/test-url/index.html.js @@ -1,15 +1,14 @@ /* eslint-disable max-len */ -import htmlMetas from "../../../../../_utils/html-metas" - export default () => ( ` - ${ htmlMetas({ - baseUrl: { pathname: "/" }, - css: [ "test.css" ] }).join("\n ") } + + + + @@ -28,7 +27,7 @@ export default () => ( "pages": {} } - + ` diff --git a/src/static/__tests__/results/destination/test/index.html.js b/src/static/__tests__/results/destination/test/index.html.js index 5a349251d..c6292ccec 100644 --- a/src/static/__tests__/results/destination/test/index.html.js +++ b/src/static/__tests__/results/destination/test/index.html.js @@ -1,15 +1,14 @@ /* eslint-disable max-len */ -import htmlMetas from "../../../../../_utils/html-metas" - export default () => ( ` - ${ htmlMetas({ - baseUrl: { pathname: "/" }, - css: [ "test.css" ] }).join("\n ") } + + + + @@ -28,7 +27,7 @@ export default () => ( "pages": {} } - + ` diff --git a/src/static/__tests__/url-as-html.js b/src/static/__tests__/url-as-html.js index 48ebc0f5e..fd825e80b 100644 --- a/src/static/__tests__/url-as-html.js +++ b/src/static/__tests__/url-as-html.js @@ -5,7 +5,6 @@ import url from "url" import test from "jest-ava-api" import beautifyHTML from "../../_utils/beautify-html" -import htmlMetas from "../../_utils/html-metas" import urlAsHtml from "../url-as-html" import collection from "./fixtures/collection.js" @@ -21,6 +20,7 @@ test("url as html", async (t) => { baseUrl: url.parse("http://0.0.0.0:3000/"), assetsFiles: { js: [ "test.js" ], + css: [ "test.css" ], }, clientScripts: true, }, @@ -32,8 +32,11 @@ test("url as html", async (t) => { - ${ htmlMetas({ baseUrl: { pathname: "/" } }).join("\n ") } + + + + @@ -56,7 +59,7 @@ test("url as html", async (t) => { } } - + `) @@ -84,8 +87,10 @@ test("url as html without JS", async (t) => { - ${ htmlMetas({ baseUrl: { pathname: "/" } }).join("\n ") } + + + @@ -124,8 +129,10 @@ test("baseUrl with offline support", async (t) => { - ${ htmlMetas({ baseUrl: { pathname: "/phenomic" } }).join("\n ") } + + + @@ -148,7 +155,7 @@ test("baseUrl with offline support", async (t) => { } } - + `) @@ -176,8 +183,10 @@ test("custom script tags", async (t) => { - ${ htmlMetas({ baseUrl: { pathname: "/" } }).join("\n ") } + + + @@ -203,8 +212,8 @@ test("custom script tags", async (t) => { } } + - `) diff --git a/src/static/url-as-html.js b/src/static/url-as-html.js index 9d13d4748..126cc6cd1 100644 --- a/src/static/url-as-html.js +++ b/src/static/url-as-html.js @@ -4,10 +4,8 @@ import React from "react" import ReactDOMserver from "react-dom/server" import { match, RouterContext as RouterContextProvider } from "react-router" import { Provider as ReduxContextProvider } from "react-redux" -import Helmet from "react-helmet" import DefaultHtml from "../components/Html" -import htmlMetas from "../_utils/html-metas" import pathToUri from "../_utils/path-to-uri" import PhenomicContextProvider from "../components/ContextProvider" import serialize from "../_utils/serialize" @@ -35,11 +33,6 @@ export default function( ] return new Promise((resolve, reject) => { - const defaultMetas = htmlMetas({ - baseUrl, - css: assetsFiles.css, - }).join("") - try { match( { @@ -48,10 +41,6 @@ export default function( basename: baseUrl.pathname, }, (error, redirectLocation, renderProps) => { - let head - let body - let script - if (error) { return reject(error) } @@ -70,10 +59,13 @@ export default function( "workaround: https://github.com/MoOx/phenomic/issues" ) } - else { - const collectionMin = minifyCollection(collection) - // render app body as "react"ified html (with data-react-id) - body = render( + + const collectionMin = minifyCollection(collection) + + /* eslint-disable react/no-multi-comp */ + + const renderBody = () => { + const body = render( ) - head = Helmet.rewind() - - const initialState = { - ...store.getState(), - // only keep current page as others are not necessary - pages: { - [url]: store.getState().pages[url], - }, - } - script = - `window.__COLLECTION__ = ${ - serialize(collectionMin) - };` + - `window.__INITIAL_STATE__ = ${ - serialize(initialState) - }` + return ( +
+ ) } - const headTags = ( - head.base.toString() + - defaultMetas + - head.meta.toString() + - head.title.toString() + - head.link.toString() - ) - - const htmlProps = { - lang: "en", - ...head.htmlAttributes.toComponent(), - } + const renderScript = () => { + if (options.clientScripts) { + const initialState = { + ...store.getState(), + // only keep current page as others are not necessary + pages: { + [url]: store.getState().pages[url], + }, + } + const script = ( + `window.__COLLECTION__ = ${ serialize(collectionMin) };` + + `window.__INITIAL_STATE__ = ${ serialize(initialState) }` + ) - const scriptTags = assetsFiles.js.map((fileName) => -