From f0afe9a4c4f9706f43488e7e48bd16956ce11f76 Mon Sep 17 00:00:00 2001 From: Arunoda Susiripala Date: Tue, 20 Dec 2016 17:48:48 +0530 Subject: [PATCH 1/3] Prevent using 'next/router' APIs inside 'getInitialProps' --- examples/using-router/components/Header.js | 1 + examples/using-router/pages/error.js | 16 ++++++++ lib/router/index.js | 46 +++++++++++++++------- server/render.js | 9 +++-- 4 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 examples/using-router/pages/error.js diff --git a/examples/using-router/components/Header.js b/examples/using-router/components/Header.js index a07ab14be6211..f762fbc300008 100644 --- a/examples/using-router/components/Header.js +++ b/examples/using-router/components/Header.js @@ -24,6 +24,7 @@ export default () => (
Home About + Error
Now you are in the route: {Router.route}
diff --git a/examples/using-router/pages/error.js b/examples/using-router/pages/error.js new file mode 100644 index 0000000000000..fb6bded69412a --- /dev/null +++ b/examples/using-router/pages/error.js @@ -0,0 +1,16 @@ +import React from 'react' +import Header from '../components/Header' +import Router from 'next/router' + +const ErrorPage = ({ aa }) => ( +
+
+

This should not be rendered via SSR

+
+) + +ErrorPage.getInitialProps = () => { + console.log(Router.pathname) +} + +export default ErrorPage diff --git a/lib/router/index.js b/lib/router/index.js index 3cfcea302e7fc..ee15a4537d47d 100644 --- a/lib/router/index.js +++ b/lib/router/index.js @@ -16,6 +16,7 @@ propertyFields.forEach((field) => { // proper way to access it Object.defineProperty(SingletonRouter, field, { get () { + throwIfNoRouter() return router[field] } }) @@ -23,29 +24,46 @@ propertyFields.forEach((field) => { methodFields.forEach((field) => { SingletonRouter[field] = (...args) => { + throwIfNoRouter() return router[field](...args) } }) -// This is an internal method and it should not be called directly. -// -// ## Client Side Usage -// We create the router in the client side only for a single time when we are -// booting the app. It happens before rendering any components. -// At the time of the component rendering, there'll be a router instance -// -// ## Server Side Usage -// We create router for every SSR page render. -// Since rendering happens in the same eventloop this works properly. +function throwIfNoRouter () { + if (!router) { + const message = 'No router instance found.\n' + + 'If you are using "next/router" inside "getInitialProps", avoid it.\n' + throw new Error(message) + } +} + +// Export the SingletonRouter and this is the public API. +// This is an client side API and doesn't available on the server +export default SingletonRouter + +// INTERNAL APIS +// ------------- +// (do not use following exports inside the app) + +// Create a router and assign it as the singleton instance. +// This is used in client side when we are initilizing the app. +// This should **not** use inside the server. export const createRouter = function (...args) { router = new _Router(...args) return router } +// This helper is used to assign a router instance only when running the "cb" +// This is useful for assigning a router instance when we do SSR. +export const withRouter = function (r, cb) { + const original = router + router = r + const result = cb() + + router = original + return result +} + // Export the actual Router class, which is also use internally // You'll ever need to access this directly export const Router = _Router - -// Export the SingletonRouter and this is the public API. -// This is an client side API and doesn't available on the server -export default SingletonRouter diff --git a/server/render.js b/server/render.js index aad29fe6e6e23..ffc03abb593b1 100644 --- a/server/render.js +++ b/server/render.js @@ -3,7 +3,7 @@ import { createElement } from 'react' import { renderToString, renderToStaticMarkup } from 'react-dom/server' import requireModule from './require' import read from './read' -import { createRouter } from '../lib/router' +import { Router, withRouter } from '../lib/router' import Head, { defaultHead } from '../lib/head' import App from '../lib/app' @@ -56,13 +56,16 @@ async function doRender (req, res, pathname, query, { if (res.finished) return const renderPage = () => { - const router = createRouter(pathname, query) + const router = new Router(pathname, query) const app = createElement(App, { Component, props, router }) - const html = (staticMarkup ? renderToStaticMarkup : renderToString)(app) + + const html = withRouter(router, () => { + return (staticMarkup ? renderToStaticMarkup : renderToString)(app) + }) const head = Head.rewind() || defaultHead() return { html, head } } From 84a4dcd67e83cd2e45ee2a90b971a945def04a0b Mon Sep 17 00:00:00 2001 From: Arunoda Susiripala Date: Tue, 20 Dec 2016 18:01:49 +0530 Subject: [PATCH 2/3] Remove incorrect documentation. --- lib/router/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/router/index.js b/lib/router/index.js index ee15a4537d47d..d5541f0a3278a 100644 --- a/lib/router/index.js +++ b/lib/router/index.js @@ -38,7 +38,6 @@ function throwIfNoRouter () { } // Export the SingletonRouter and this is the public API. -// This is an client side API and doesn't available on the server export default SingletonRouter // INTERNAL APIS From dd33235de6380b9eacf8c2c2d8bb8ba12b72a7c1 Mon Sep 17 00:00:00 2001 From: Arunoda Susiripala Date: Tue, 20 Dec 2016 18:50:37 +0530 Subject: [PATCH 3/3] Make next/router a client only API. --- examples/using-router/components/Header.js | 3 --- lib/router/index.js | 18 +++--------------- server/render.js | 9 +++------ 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/examples/using-router/components/Header.js b/examples/using-router/components/Header.js index f762fbc300008..0a4da90ba7d1c 100644 --- a/examples/using-router/components/Header.js +++ b/examples/using-router/components/Header.js @@ -25,8 +25,5 @@ export default () => ( Home About Error -
- Now you are in the route: {Router.route} -
) diff --git a/lib/router/index.js b/lib/router/index.js index d5541f0a3278a..20c9e7e6ba36f 100644 --- a/lib/router/index.js +++ b/lib/router/index.js @@ -6,7 +6,7 @@ let router = null const SingletonRouter = {} // Create public properties and methods of the router in the SingletonRouter -const propertyFields = ['route', 'components', 'pathname', 'query'] +const propertyFields = ['components', 'pathname', 'route', 'query'] const methodFields = ['push', 'replace', 'reload', 'back'] propertyFields.forEach((field) => { @@ -32,7 +32,7 @@ methodFields.forEach((field) => { function throwIfNoRouter () { if (!router) { const message = 'No router instance found.\n' + - 'If you are using "next/router" inside "getInitialProps", avoid it.\n' + 'You should only use "next/router" inside the client side of your app.\n' throw new Error(message) } } @@ -52,17 +52,5 @@ export const createRouter = function (...args) { return router } -// This helper is used to assign a router instance only when running the "cb" -// This is useful for assigning a router instance when we do SSR. -export const withRouter = function (r, cb) { - const original = router - router = r - const result = cb() - - router = original - return result -} - -// Export the actual Router class, which is also use internally -// You'll ever need to access this directly +// Export the actual Router class, which is usually used inside the server export const Router = _Router diff --git a/server/render.js b/server/render.js index ffc03abb593b1..25facc0341edc 100644 --- a/server/render.js +++ b/server/render.js @@ -3,7 +3,7 @@ import { createElement } from 'react' import { renderToString, renderToStaticMarkup } from 'react-dom/server' import requireModule from './require' import read from './read' -import { Router, withRouter } from '../lib/router' +import { Router } from '../lib/router' import Head, { defaultHead } from '../lib/head' import App from '../lib/app' @@ -56,16 +56,13 @@ async function doRender (req, res, pathname, query, { if (res.finished) return const renderPage = () => { - const router = new Router(pathname, query) const app = createElement(App, { Component, props, - router + router: new Router(pathname, query) }) - const html = withRouter(router, () => { - return (staticMarkup ? renderToStaticMarkup : renderToString)(app) - }) + const html = (staticMarkup ? renderToStaticMarkup : renderToString)(app) const head = Head.rewind() || defaultHead() return { html, head } }