commit 35b7c774f084a5f46fe185d93f06f3123b815d25 parent be660f6d5a413078a83bdd2ce8f946b2dfef0147 Author: Vladimir Kutepov <frenzzy.man@gmail.com> Date: Tue, 13 Sep 2016 13:18:09 +0400 Improve flexibility of routing logic (#849) Diffstat:
22 files changed, 278 insertions(+), 191 deletions(-)
diff --git a/package.json b/package.json @@ -64,11 +64,11 @@ "del": "^2.2.2", "enzyme": "^2.4.1", "eslint": "^3.4.0", - "eslint-config-airbnb": "^11.0.0", + "eslint-config-airbnb": "^11.1.0", "eslint-loader": "^1.5.0", "eslint-plugin-import": "^1.14.0", - "eslint-plugin-jsx-a11y": "^2.2.1", - "eslint-plugin-react": "^6.2.0", + "eslint-plugin-jsx-a11y": "^2.2.2", + "eslint-plugin-react": "^6.2.1", "extend": "^3.0.0", "file-loader": "^0.9.0", "gaze": "^1.1.1", @@ -92,7 +92,7 @@ "postcss-media-minmax": "^2.1.2", "postcss-nesting": "^2.3.1", "postcss-pseudoelements": "^3.0.0", - "postcss-selector-matches": "^2.0.4", + "postcss-selector-matches": "^2.0.5", "postcss-selector-not": "^2.0.0", "raw-loader": "^0.5.1", "react-addons-test-utils": "15.3.1", @@ -131,6 +131,7 @@ "browser": true }, "rules": { + "arrow-parens": "off", "generator-star-spacing": "off", "import/no-extraneous-dependencies": "off", "react/forbid-prop-types": "off", diff --git a/src/client.js b/src/client.js @@ -12,7 +12,6 @@ import ReactDOM from 'react-dom'; import FastClick from 'fastclick'; import UniversalRouter from 'universal-router'; import { readState, saveState } from 'history/lib/DOMStateStorage'; -import routes from './routes'; import history from './core/history'; import { addEventListener, @@ -28,25 +27,33 @@ const context = { removeCss.forEach(f => f()); }; }, - setTitle: value => (document.title = value), - setMeta: (name, content) => { - // Remove and create a new <meta /> tag in order to make it work - // with bookmarks in Safari - const elements = document.getElementsByTagName('meta'); - Array.from(elements).forEach((element) => { - if (element.getAttribute('name') === name) { - element.parentNode.removeChild(element); - } - }); - const meta = document.createElement('meta'); - meta.setAttribute('name', name); - meta.setAttribute('content', content); - document - .getElementsByTagName('head')[0] - .appendChild(meta); - }, }; +function updateTag(tag, nameKey, valueKey, name, value) { + // Remove and create a new <meta /> tag in order to make it work with bookmarks in Safari + let meta = document.head.querySelector(`${tag}[${nameKey}=${name}]`); + if (!meta || meta.getAttribute(valueKey) !== value) { + if (meta) { + meta.parentNode.removeChild(meta); + } + if (typeof value === 'string') { + meta = document.createElement(tag); + meta.setAttribute(nameKey, name); + meta.setAttribute(valueKey, value); + document.head.appendChild(meta); + } + } +} +function updateMeta(name, value) { + updateTag('meta', 'name', 'content', name, value); +} +function updateLink(name, value) { // eslint-disable-line no-unused-vars + updateTag('link', 'rel', 'href', name, value); +} +function updateCustomMeta(name, value) { // eslint-disable-line no-unused-vars + updateTag('meta', 'property', 'content', name, value); +} + // Restore the scroll position if it was saved into the state function restoreScrollPosition({ state, hash }) { if (state && state.scrollY !== undefined) { @@ -66,30 +73,38 @@ function restoreScrollPosition({ state, hash }) { window.scrollTo(0, 0); } -let renderComplete = (location, callback) => { +let onRenderComplete = function initialRenderComplete() { const elem = document.getElementById('css'); if (elem) elem.parentNode.removeChild(elem); - callback(true); - renderComplete = (l) => { - restoreScrollPosition(l); + onRenderComplete = function renderComplete(route, location) { + document.title = route.title; + + updateMeta('description', route.description); + // Update necessary custom tags at runtime here, ie: + // updateMeta('keywords', route.keywords); + // updateLink('canonical', route.canonicalUrl); + // updateCustomMeta('og:url', route.canonicalUrl); + // updateCustomMeta('og:image', route.imageUrl); + // etc. + + restoreScrollPosition(location); // Google Analytics tracking. Don't send 'pageview' event after // the initial rendering, as it was already sent if (window.ga) { - window.ga('send', 'pageview'); + window.ga('send', 'pageview', location.pathname + location.search); } - - callback(true); }; }; -function render(container, location, component) { +const container = document.getElementById('app'); +function render(route, location) { return new Promise((resolve, reject) => { try { ReactDOM.render( - component, + route.component, container, - renderComplete.bind(undefined, location, resolve) + onRenderComplete.bind(undefined, route, location) ); } catch (err) { reject(err); @@ -97,59 +112,66 @@ function render(container, location, component) { }); } -function run() { - const container = document.getElementById('app'); - let currentLocation = history.getCurrentLocation(); - - // Make taps on links and buttons work fast on mobiles - FastClick.attach(document.body); - - // Re-render the app when window.location changes - function onLocationChange(location) { - // Save the page scroll position into the current location's state - if (currentLocation.key) { - saveState(currentLocation.key, { - ...readState(currentLocation.key), - scrollX: windowScrollX(), - scrollY: windowScrollY(), - }); - } - currentLocation = location; +// Make taps on links and buttons work fast on mobiles +FastClick.attach(document.body); - UniversalRouter.resolve(routes, { +let currentLocation = history.getCurrentLocation(); +let routes = require('./routes').default; + +// Re-render the app when window.location changes +async function onLocationChange(location) { + // Save the page scroll position into the current location's state + if (currentLocation.key) { + saveState(currentLocation.key, { + ...readState(currentLocation.key), + scrollX: windowScrollX(), + scrollY: windowScrollY(), + }); + } + currentLocation = location; + + try { + const route = await UniversalRouter.resolve(routes, { path: location.pathname, query: location.query, state: location.state, context, - render: render.bind(undefined, container, location), - }).catch(err => console.error(err)); // eslint-disable-line no-console + }); + + await render(route, location); + } catch (err) { + // TODO: Inform the user about the failed page transition. + console.error(err); // eslint-disable-line no-console } +} - // Add History API listener and trigger initial change - const removeHistoryListener = history.listen(onLocationChange); - history.replace(currentLocation); +// Add History API listener and trigger initial change +const removeHistoryListener = history.listen(onLocationChange); +history.replace(currentLocation); - // https://developers.google.com/web/updates/2015/09/history-api-scroll-restoration - let originalScrollRestoration; - if (window.history && 'scrollRestoration' in window.history) { - originalScrollRestoration = window.history.scrollRestoration; - window.history.scrollRestoration = 'manual'; +// Switch off the native scroll restoration behavior and handle it manually +// https://developers.google.com/web/updates/2015/09/history-api-scroll-restoration +let originalScrollRestoration; +if (window.history && 'scrollRestoration' in window.history) { + originalScrollRestoration = window.history.scrollRestoration; + window.history.scrollRestoration = 'manual'; +} + +// Prevent listeners collisions during history navigation +addEventListener(window, 'pagehide', function onPageHide() { + removeEventListener(window, 'pagehide', onPageHide); + removeHistoryListener(); + if (originalScrollRestoration) { + window.history.scrollRestoration = originalScrollRestoration; + originalScrollRestoration = undefined; } +}); - // Prevent listeners collisions during history navigation - addEventListener(window, 'pagehide', function onPageHide() { - removeEventListener(window, 'pagehide', onPageHide); - removeHistoryListener(); - if (originalScrollRestoration) { - window.history.scrollRestoration = originalScrollRestoration; - originalScrollRestoration = undefined; - } - }); -} +// Enable Hot Module Replacement (HMR) +if (module.hot) { + module.hot.accept('./routes', () => { + routes = require('./routes').default; // eslint-disable-line global-require -// Run the application when both DOM is ready and page content is loaded -if (['complete', 'loaded', 'interactive'].includes(document.readyState) && document.body) { - run(); -} else { - document.addEventListener('DOMContentLoaded', run, false); + onLocationChange(history.getCurrentLocation()); + }); } diff --git a/src/components/Html.js b/src/components/Html.js @@ -11,7 +11,7 @@ function Html({ title, description, style, script, children }) { <meta name="description" content={description} /> <meta name="viewport" content="width=device-width, initial-scale=1" /> <link rel="apple-touch-icon" href="apple-touch-icon.png" /> - <style id="css" dangerouslySetInnerHTML={{ __html: style }} /> + {style && <style id="css" dangerouslySetInnerHTML={{ __html: style }} />} </head> <body> <div id="app" dangerouslySetInnerHTML={{ __html: children }} /> @@ -32,11 +32,16 @@ function Html({ title, description, style, script, children }) { } Html.propTypes = { - title: PropTypes.string.isRequired, - description: PropTypes.string.isRequired, - style: PropTypes.string.isRequired, + title: PropTypes.string, + description: PropTypes.string, + style: PropTypes.string, script: PropTypes.string, children: PropTypes.string, }; +Html.defaultProps = { + title: '', + description: '', +}; + export default Html; diff --git a/src/routes/contact/Contact.js b/src/routes/contact/Contact.js @@ -11,10 +11,7 @@ import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './Contact.css'; -const title = 'Contact Us'; - -function Contact(props, context) { - context.setTitle(title); +function Contact({ title }) { return ( <div className={s.root}> <div className={s.container}> @@ -25,6 +22,6 @@ function Contact(props, context) { ); } -Contact.contextTypes = { setTitle: PropTypes.func.isRequired }; +Contact.propTypes = { title: PropTypes.string.isRequired }; export default withStyles(s)(Contact); diff --git a/src/routes/contact/index.js b/src/routes/contact/index.js @@ -10,12 +10,17 @@ import React from 'react'; import Contact from './Contact'; +const title = 'Contact Us'; + export default { path: '/contact', action() { - return <Contact />; + return { + title, + component: <Contact title={title} />, + }; }, }; diff --git a/src/routes/content/Content.js b/src/routes/content/Content.js @@ -7,41 +7,25 @@ * LICENSE.txt file in the root directory of this source tree. */ -import React, { Component, PropTypes } from 'react'; +import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './Content.css'; -class Content extends Component { - - static contextTypes = { - setTitle: PropTypes.func.isRequired, - }; - - static propTypes = { - path: PropTypes.string.isRequired, - content: PropTypes.string.isRequired, - title: PropTypes.string, - }; - - componentWillMount() { - this.context.setTitle(this.props.title); - } - - componentWillReceiveProps(nextProps) { - this.context.setTitle(nextProps.title); - } - - render() { - return ( - <div className={s.root}> - <div className={s.container}> - {this.props.path === '/' ? null : <h1>{this.props.title}</h1>} - <div dangerouslySetInnerHTML={{ __html: this.props.content || '' }} /> - </div> +function Content({ path, title, content }) { + return ( + <div className={s.root}> + <div className={s.container}> + {title && path !== '/' && <h1>{title}</h1>} + <div dangerouslySetInnerHTML={{ __html: content }} /> </div> - ); - } - + </div> + ); } +Content.propTypes = { + path: PropTypes.string.isRequired, + content: PropTypes.string.isRequired, + title: PropTypes.string, +}; + export default withStyles(s)(Content); diff --git a/src/routes/content/index.js b/src/routes/content/index.js @@ -30,7 +30,10 @@ export default { if (resp.status !== 200) throw new Error(resp.statusText); const { data } = await resp.json(); if (!data || !data.content) return undefined; - return <Content {...data.content} />; + return { + title: data.content.title, + component: <Content {...data.content} />, + }; }, }; diff --git a/src/routes/error/ErrorPage.css b/src/routes/error/ErrorPage.css @@ -24,9 +24,7 @@ html { body { display: table-cell; vertical-align: middle; - /* stylelint-disable */ - margin: 2em auto; - /* stylelint-enable */ + padding: 2em; } h1 { diff --git a/src/routes/error/ErrorPage.js b/src/routes/error/ErrorPage.js @@ -11,33 +11,26 @@ import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './ErrorPage.css'; -function ErrorPage({ error }, context) { - let title = 'Error'; - let content = 'Sorry, a critical error occurred on this page.'; - let errorMessage = null; - - if (error.status === 404) { - title = 'Page Not Found'; - content = 'Sorry, the page you were trying to view does not exist.'; - } else if (process.env.NODE_ENV !== 'production') { - errorMessage = <pre>{error.stack}</pre>; - } - - if (context.setTitle) { - context.setTitle(title); +function ErrorPage({ error }) { + if (process.env.NODE_ENV === 'production') { + return ( + <div> + <h1>Error</h1> + <p>Sorry, a critical error occurred on this page.</p> + </div> + ); } return ( <div> - <h1>{title}</h1> - <p>{content}</p> - {errorMessage} + <h1>{error.name}</h1> + <p>{error.message}</p> + <pre>{error.stack}</pre> </div> ); } ErrorPage.propTypes = { error: PropTypes.object.isRequired }; -ErrorPage.contextTypes = { setTitle: PropTypes.func.isRequired }; export { ErrorPage as ErrorPageWithoutStyle }; export default withStyles(s)(ErrorPage); diff --git a/src/routes/error/index.js b/src/routes/error/index.js @@ -8,20 +8,19 @@ */ import React from 'react'; -import App from '../../components/App'; import ErrorPage from './ErrorPage'; export default { path: '/error', - action({ render, context, error }) { - return render( - <App context={context} error={error}> - <ErrorPage error={error} /> - </App>, - error.status || 500 - ); + action({ error }) { + return { + title: error.name, + description: error.message, + component: <ErrorPage error={error} />, + status: error.status || 500, + }; }, }; diff --git a/src/routes/home/Home.js b/src/routes/home/Home.js @@ -11,10 +11,7 @@ import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './Home.css'; -const title = 'React Starter Kit'; - -function Home({ news }, context) { - context.setTitle(title); +function Home({ news }) { return ( <div className={s.root}> <div className={s.container}> @@ -42,6 +39,5 @@ Home.propTypes = { contentSnippet: PropTypes.string, })).isRequired, }; -Home.contextTypes = { setTitle: PropTypes.func.isRequired }; export default withStyles(s)(Home); diff --git a/src/routes/home/index.js b/src/routes/home/index.js @@ -29,7 +29,10 @@ export default { }); const { data } = await resp.json(); if (!data || !data.news) throw new Error('Failed to load the news feed.'); - return <Home news={data.news} />; + return { + title: 'React Starter Kit', + component: <Home news={data.news} />, + }; }, }; diff --git a/src/routes/index.js b/src/routes/index.js @@ -16,7 +16,7 @@ import contact from './contact'; import login from './login'; import register from './register'; import content from './content'; -import error from './error'; +import notFound from './notFound'; export default { @@ -31,15 +31,26 @@ export default { // place new routes before... content, - error, + notFound, ], - async action({ next, render, context }) { - const component = await next(); - if (component === undefined) return component; - return render( - <App context={context}>{component}</App> - ); + async action({ next, context }) { + let route; + + // Execute each child route until one of them return the result + // TODO: move this logic to the `next` function + do { + route = await next(); + } while (!route); + + return { + ...route, + + // Override the result of child route with extensions + title: `${route.title || 'Untitled Page'} - www.reactstarterkit.com`, + description: route.description || '', + component: <App context={context}>{route.component}</App>, + }; }, }; diff --git a/src/routes/login/Login.js b/src/routes/login/Login.js @@ -11,10 +11,7 @@ import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './Login.css'; -const title = 'Log In'; - -function Login(props, context) { - context.setTitle(title); +function Login({ title }) { return ( <div className={s.root}> <div className={s.container}> @@ -117,6 +114,6 @@ function Login(props, context) { ); } -Login.contextTypes = { setTitle: PropTypes.func.isRequired }; +Login.propTypes = { title: PropTypes.string.isRequired }; export default withStyles(s)(Login); diff --git a/src/routes/login/index.js b/src/routes/login/index.js @@ -10,12 +10,17 @@ import React from 'react'; import Login from './Login'; +const title = 'Log In'; + export default { path: '/login', action() { - return <Login />; + return { + title, + component: <Login title={title} />, + }; }, }; diff --git a/src/routes/notFound/NotFound.css b/src/routes/notFound/NotFound.css @@ -0,0 +1,21 @@ +/** + * React Starter Kit (https://www.reactstarterkit.com/) + * + * Copyright © 2014-2016 Kriasoft, LLC. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE.txt file in the root directory of this source tree. + */ + +@import '../../components/variables.css'; + +.root { + padding-left: 20px; + padding-right: 20px; +} + +.container { + margin: 0 auto; + padding: 0 0 40px; + max-width: var(--max-content-width); +} diff --git a/src/routes/notFound/NotFound.js b/src/routes/notFound/NotFound.js @@ -0,0 +1,27 @@ +/** + * React Starter Kit (https://www.reactstarterkit.com/) + * + * Copyright © 2014-2016 Kriasoft, LLC. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE.txt file in the root directory of this source tree. + */ + +import React, { PropTypes } from 'react'; +import withStyles from 'isomorphic-style-loader/lib/withStyles'; +import s from './NotFound.css'; + +function NotFound({ title }) { + return ( + <div className={s.root}> + <div className={s.container}> + <h1>{title}</h1> + <p>Sorry, the page you were trying to view does not exist.</p> + </div> + </div> + ); +} + +NotFound.propTypes = { title: PropTypes.string.isRequired }; + +export default withStyles(s)(NotFound); diff --git a/src/routes/notFound/index.js b/src/routes/notFound/index.js @@ -0,0 +1,27 @@ +/** + * React Starter Kit (https://www.reactstarterkit.com/) + * + * Copyright © 2014-2016 Kriasoft, LLC. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE.txt file in the root directory of this source tree. + */ + +import React from 'react'; +import NotFound from './NotFound'; + +const title = 'Page Not Found'; + +export default { + + path: '*', + + action() { + return { + title, + component: <NotFound title={title} />, + status: 404, + }; + }, + +}; diff --git a/src/routes/register/Register.js b/src/routes/register/Register.js @@ -11,10 +11,7 @@ import React, { PropTypes } from 'react'; import withStyles from 'isomorphic-style-loader/lib/withStyles'; import s from './Register.css'; -const title = 'New User Registration'; - -function Register(props, context) { - context.setTitle(title); +function Register({ title }) { return ( <div className={s.root}> <div className={s.container}> @@ -25,6 +22,6 @@ function Register(props, context) { ); } -Register.contextTypes = { setTitle: PropTypes.func.isRequired }; +Register.propTypes = { title: PropTypes.string.isRequired }; export default withStyles(s)(Register); diff --git a/src/routes/register/index.js b/src/routes/register/index.js @@ -10,12 +10,17 @@ import React from 'react'; import Register from './Register'; +const title = 'New User Registration'; + export default { path: '/register', action() { - return <Register />; + return { + title, + component: <Register title={title} />, + }; }, }; diff --git a/src/server.js b/src/server.js @@ -84,32 +84,24 @@ app.use('/graphql', expressGraphQL(req => ({ // ----------------------------------------------------------------------------- app.get('*', async (req, res, next) => { try { - let css = new Set(); - let statusCode = 200; - const data = { title: '', description: '', style: '', script: assets.main.js, children: '' }; - - await UniversalRouter.resolve(routes, { + const css = new Set(); + const route = await UniversalRouter.resolve(routes, { path: req.path, query: req.query, context: { insertCss: (...styles) => { styles.forEach(style => css.add(style._getCss())); // eslint-disable-line no-underscore-dangle, max-len }, - setTitle: value => (data.title = value), - setMeta: (key, value) => (data[key] = value), - }, - render(component, status = 200) { - css = new Set(); - statusCode = status; - data.children = ReactDOM.renderToString(component); - data.style = [...css].join(''); - return true; }, }); + const data = { ...route }; + data.children = ReactDOM.renderToString(route.component); + data.style = [...css].join(''); + data.script = assets.main.js; const html = ReactDOM.renderToStaticMarkup(<Html {...data} />); - res.status(statusCode); + res.status(route.status || 200); res.send(`<!doctype html>${html}`); } catch (err) { next(err); @@ -125,7 +117,6 @@ pe.skipPackage('express'); app.use((err, req, res, next) => { // eslint-disable-line no-unused-vars console.log(pe.render(err)); // eslint-disable-line no-console - const statusCode = err.status || 500; const html = ReactDOM.renderToStaticMarkup( <Html title="Internal Server Error" @@ -135,7 +126,7 @@ app.use((err, req, res, next) => { // eslint-disable-line no-unused-vars {ReactDOM.renderToString(<ErrorPageWithoutStyle error={err} />)} </Html> ); - res.status(statusCode); + res.status(err.status || 500); res.send(`<!doctype html>${html}`); }); diff --git a/tools/webpack.config.js b/tools/webpack.config.js @@ -252,7 +252,7 @@ const clientConfig = extend(true, {}, config, { // Choose a developer tool to enhance debugging // http://webpack.github.io/docs/configuration.html#devtool - devtool: DEBUG ? 'cheap-module-eval-source-map' : false, + devtool: DEBUG ? 'source-map' : false, }); //