From aee67c51b71446fbc0948b5d76fbd5614aee3783 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 10:15:45 -0400 Subject: [PATCH 01/14] Add onDispatch callback --- lib/RoutingEnvironmentMixin.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/RoutingEnvironmentMixin.js b/lib/RoutingEnvironmentMixin.js index 86e9eb6..01e92e1 100644 --- a/lib/RoutingEnvironmentMixin.js +++ b/lib/RoutingEnvironmentMixin.js @@ -28,7 +28,8 @@ var RoutingEnvironmentMixin = { hash: React.PropTypes.bool, onBeforeNavigation: React.PropTypes.func, - onNavigation: React.PropTypes.func + onNavigation: React.PropTypes.func, + onDispatch: React.PropTypes.func }, childContextTypes: { @@ -166,6 +167,12 @@ var RoutingEnvironmentMixin = { return join(this.getPrefix(), href); }, + componentWillMount: function() { + if (this.props.onDispatch) { + this.props.onDispatch(this.getPath(), {}); + } + }, + componentDidMount: function() { if (this.isControlled()) { return; @@ -226,9 +233,14 @@ EnvironmentListener.prototype.onBeforeNavigation = function(path, navigation) { } EnvironmentListener.prototype.onNavigation = function(path, navigation) { + var result; if (this.router.props.onNavigation) { - return this.router.props.onNavigation(path, navigation); + result = this.router.props.onNavigation(path, navigation); + } + if (this.router.props.onDispatch) { + this.router.props.onDispatch(path, navigation); } + return result; } module.exports = RoutingEnvironmentMixin; From 3857d8f4c796ad7e1da604932c9061642dfe5214 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 10:15:57 -0400 Subject: [PATCH 02/14] Test onDispatch callback for initial path --- tests/server.js | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/server.js b/tests/server.js index fa016ff..8d1f781 100644 --- a/tests/server.js +++ b/tests/server.js @@ -4,10 +4,26 @@ var Router = require('../index'); describe('react-router-component (on server)', function() { + var dispatchCount; + var dispatchPath; + + beforeEach(function() { + dispatchCount = 0; + dispatchPath = null; + }); + var App = React.createClass({ + handleDispatch: function(path) { + dispatchCount += 1; + dispatchPath = path; + }, + render: function() { - return Router.Locations({className: 'App', path: this.props.path}, + return Router.Locations({ + className: 'App', + path: this.props.path, + onDispatch: this.handleDispatch}, Router.Location({ path: '/', handler: function(props) { return React.DOM.div(null, 'mainpage'); } @@ -41,6 +57,16 @@ describe('react-router-component (on server)', function() { assert(markup.match(/not_found/)); }); + it('invokes the onDispatch callback with the initial path', function() { + React.renderComponentToString(App({path: '/x/hello'})); + assert.equal(dispatchPath, '/x/hello'); + }); + + it('invokes the onDispatch callback once for the initial path', function() { + React.renderComponentToString(App()); + assert.equal(dispatchCount, 1); + }); + describe('pages router', function() { var App = React.createClass({ From 6a4856f045415b42e39f1d7eb983fba5d3ac90f9 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 10:49:27 -0400 Subject: [PATCH 03/14] Test onDispatch callback for navigate calls --- tests/browser.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/browser.js b/tests/browser.js index 8e6b37a..b49338e 100644 --- a/tests/browser.js +++ b/tests/browser.js @@ -74,7 +74,8 @@ describe('Routing', function() { Router.Locations({ ref: 'router', className: 'App', onNavigation: this.props.navigationHandler, - onBeforeNavigation: this.props.beforeNavigationHandler + onBeforeNavigation: this.props.beforeNavigationHandler, + onDispatch: this.props.dispatchHandler }, Router.Location({ path: '/__zuul', @@ -184,7 +185,7 @@ describe('Routing', function() { }); describe('Navigation lifecycle callbacks', function () { - it('calls onBeforeNaviation and onNavigation', function(done) { + it('calls onBeforeNaviation, onNavigation, and onDispatch', function(done) { assertRendered('mainpage'); var called = []; app.setProps({ @@ -193,12 +194,16 @@ describe('Routing', function() { }, navigationHandler: function () { called.push('onNavigation'); + }, + dispatchHandler: function (path) { + called.push(path) } }); router.navigate('/__zuul/hello', function () { - assert.equal(called.length, 2); + assert.equal(called.length, 3); assert.equal(called[0], '/__zuul/hello'); assert.equal(called[1], 'onNavigation'); + assert.equal(called[2], '/__zuul/hello'); done(); }); }); From 1a718621e4d1d84af744279f8dc041615398c634 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 14:22:55 -0400 Subject: [PATCH 04/14] Pass extra info to onDispatch --- lib/RoutingEnvironmentMixin.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/RoutingEnvironmentMixin.js b/lib/RoutingEnvironmentMixin.js index 01e92e1..67bbf3f 100644 --- a/lib/RoutingEnvironmentMixin.js +++ b/lib/RoutingEnvironmentMixin.js @@ -169,7 +169,12 @@ var RoutingEnvironmentMixin = { componentWillMount: function() { if (this.props.onDispatch) { - this.props.onDispatch(this.getPath(), {}); + var path = this.getPath(); + this.props.onDispatch(path, { + initial: true, + isPopState: false, + match: this.match(path) + }); } }, From 064bb1cdabfa1937aa917fc5df1a4e413ae81489 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 14:24:50 -0400 Subject: [PATCH 05/14] Add connect middleware Woah, cool! --- lib/contrib/connect.js | 89 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 lib/contrib/connect.js diff --git a/lib/contrib/connect.js b/lib/contrib/connect.js new file mode 100644 index 0000000..41ed85c --- /dev/null +++ b/lib/contrib/connect.js @@ -0,0 +1,89 @@ +"use strict"; + +var URL = require('url'); +var React = require('react'); +var merge = require('react/lib/merge'); +var invariant = require('react/lib/invariant'); + + +function getDoctype(nameOrValue) { + if (!nameOrValue){ + return ''; + } + switch (nameOrValue.toString().toLowerCase()) { + case 'default': + case '5': + case 'html': + case 'html5': + return ''; + case '4': + case 'html4': + return ''; + case 'transitional': + return ''; + case 'strict': + return ''; + case 'frameset': + return ''; + case '1.1': + return ''; + case 'basic': + return ''; + case 'mobile': + return ''; + } + return nameOrValue; +} + + +/** + * A connect middleware for server-side rendering of React Router components. + * Typically, these are instances of Router, but the only requirements are that + * the component accepts "path" on "onDispatch" props. + */ +function reactRouter(Router, options) { + invariant( + React.isValidClass(Router), + 'Argument must be a React component class but got: %s', Router + ); + var doctype = getDoctype(options && options.doctype); + var props = options && options.props || {}; + return function(req, res, next) { + var pathname = URL.parse(req.url).pathname; + if (pathname == null) { + next(new Error('Invalid URL: ' + req.url)); + return; + } + + var statusCode; + var rendered; + + var callback = function() { + // Wait for the render to complete and onDispatch to be invoked. + if (rendered == null || statusCode == null) { + return; + } + res.statusCode = statusCode; + res.end('' + doctype + rendered); + }; + + try { + var app = Router(merge(props, { + path: pathname, + onDispatch: function(path, navigation) { + statusCode = (navigation && navigation.match && + navigation.match.isNotFound ? 404 : 200); + callback(); + } + })); + rendered = React.renderComponentToString(app); + callback(); + } catch (err) { + next(err); + } + }; +} + +module.exports = { + reactRouter: reactRouter +}; From aa32840ba845ab4cd5734d10c2d44fbe9f2f48d2 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 16:21:20 -0400 Subject: [PATCH 06/14] Add getContentMeta util --- lib/getContentMeta.js | 68 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 lib/getContentMeta.js diff --git a/lib/getContentMeta.js b/lib/getContentMeta.js new file mode 100644 index 0000000..f923272 --- /dev/null +++ b/lib/getContentMeta.js @@ -0,0 +1,68 @@ +"use strict"; + +var doctypes = { + 'text/html': { + '': ['default', '5', 'html', 'html5'], + '': ['4', 'html4'] + }, + 'application/xhtml+xml': { + '': ['strict'], + '': ['transitional'], + '': ['frameset'], + '': ['1.1'], + '': ['basic'], + '': ['mobile'] + }, + 'image/svg+xml': { + '': ['svg'] // https://jwatt.org/svg/authoring/#doctype-declaration + } +}; + + +function getContentMeta(doctype, contentType) { + var ct, dt, contentTypesToDoctypes, doctypesToShortcuts, shortcuts; + + if (doctype == null && contentType == null) { + doctype = 'default'; + } + + // Look up the content type + if (doctype) { + for (ct in doctypes) { + doctypesToShortcuts = doctypes[ct]; + if (!doctypesToShortcuts) { continue; } + + for (dt in doctypesToShortcuts) { + shortcuts = doctypesToShortcuts[dt]; + if (!shortcuts) { continue; } + if (String(doctype).toLowerCase() === dt.toLowerCase() || shortcuts.indexOf(String(doctype).toLowerCase()) > -1) { + return {doctype: dt, contentType: contentType == null ? ct: contentType}; + } + } + } + } + + if (contentType == null) { + throw new Error('Could not find matching content type for doctype: ' + doctype); + } + + // Look up the doctype. + for (ct in doctypes) { + if (contentType.toLowerCase() === ct.toLowerCase()) { + doctypesToShortcuts = doctypes[ct]; + if (!doctypesToShortcuts) { continue; } + for (dt in doctypesToShortcuts) { + return {doctype: dt, contentType: contentType}; + } + } + } + + if (doctype == null) { + throw new Error('Could not find matching doctype type for content type: ' + contentType); + } + + return {doctype: doctype, contentType: contentType}; +} + + +module.exports = getContentMeta; From 78815c89781a1d3271f3136b777fab5bd025fca9 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 16:21:35 -0400 Subject: [PATCH 07/14] Test getContentMeta --- Makefile | 2 +- tests/getContentMeta.js | 102 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/getContentMeta.js diff --git a/Makefile b/Makefile index 146e73a..11a71e4 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ lint: test: test-unit test-server test-unit: - @mocha -R spec -b tests/matchRoutes.js + @mocha -R spec -b tests/matchRoutes.js tests/getContentMeta.js test-server: @mocha -R spec -b tests/server.js diff --git a/tests/getContentMeta.js b/tests/getContentMeta.js new file mode 100644 index 0000000..abbdc93 --- /dev/null +++ b/tests/getContentMeta.js @@ -0,0 +1,102 @@ +var assert = require('assert'); +var getContentMeta = require('../lib/getContentMeta'); + +describe('getContentMeta', function() { + + it('finds doctype by content type', function() { + assert.strictEqual( + getContentMeta(null, 'text/html').doctype, + '' + ); + assert.strictEqual( + getContentMeta(null, 'application/xhtml+xml').doctype, + '' + ); + assert.strictEqual( + getContentMeta(null, 'image/svg+xml').doctype, + '' + ); + }); + + it('finds content type by full doctype', function() { + assert.strictEqual( + getContentMeta( + '' + ).contentType, + 'text/html' + ); + }); + + it("doesn't find a match for an empty doctype", function() { // Even though it matches SVG + assert.throws( + function() { getContentMeta(''); }, + /Could not find matching content type/ + ); + }); + + it("doesn't find a match for an nonsense doctype", function() { + assert.throws( + function() { getContentMeta('missingno'); }, + /Could not find matching content type/ + ); + }); + + it("doesn't find a match for an nonsense content type", function() { + assert.throws( + function() { getContentMeta(null, 'missingno'); }, + /Could not find matching doctype/ + ); + }); + + it('finds content type by doctype nickname', function() { + assert.strictEqual( + getContentMeta(5).contentType, + 'text/html' + ); + assert.strictEqual( + getContentMeta('html5').contentType, + 'text/html' + ); + assert.strictEqual( + getContentMeta('svg').contentType, + 'image/svg+xml' + ); + }); + + it('finds actual doctype by doctype nickname', function() { + assert.strictEqual( + getContentMeta(4).doctype, + '' + ); + assert.strictEqual( + getContentMeta('svg').doctype, + '' + ); + assert.strictEqual( + getContentMeta(4, 'text/html').doctype, + '' + ); + }); + + it('defaults to html5', function() { + assert.deepEqual( + getContentMeta(), + {doctype: '', contentType: 'text/html'} + ); + }); + + it('defaults to html5', function() { + assert.deepEqual( + getContentMeta(), + {doctype: '', contentType: 'text/html'} + ); + }); + + it('passes through explicit values', function() { + assert.deepEqual( + getContentMeta('hello', 'goodbye'), + {doctype: 'hello', contentType: 'goodbye'} + ); + }); + +}); From 9f69052a9817a52f8e834fb89bdd92be1336e1af Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 16:21:48 -0400 Subject: [PATCH 08/14] Use getContentMeta; set Content-Type header --- lib/contrib/connect.js | 45 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 36 deletions(-) diff --git a/lib/contrib/connect.js b/lib/contrib/connect.js index 41ed85c..54e9c11 100644 --- a/lib/contrib/connect.js +++ b/lib/contrib/connect.js @@ -1,39 +1,10 @@ "use strict"; -var URL = require('url'); -var React = require('react'); -var merge = require('react/lib/merge'); -var invariant = require('react/lib/invariant'); - - -function getDoctype(nameOrValue) { - if (!nameOrValue){ - return ''; - } - switch (nameOrValue.toString().toLowerCase()) { - case 'default': - case '5': - case 'html': - case 'html5': - return ''; - case '4': - case 'html4': - return ''; - case 'transitional': - return ''; - case 'strict': - return ''; - case 'frameset': - return ''; - case '1.1': - return ''; - case 'basic': - return ''; - case 'mobile': - return ''; - } - return nameOrValue; -} +var URL = require('url'); +var React = require('react'); +var merge = require('react/lib/merge'); +var invariant = require('react/lib/invariant'); +var getContentMeta = require('../getContentMeta'); /** @@ -46,7 +17,8 @@ function reactRouter(Router, options) { React.isValidClass(Router), 'Argument must be a React component class but got: %s', Router ); - var doctype = getDoctype(options && options.doctype); + var meta = getContentMeta(options && options.doctype, + options && options.contentType); var props = options && options.props || {}; return function(req, res, next) { var pathname = URL.parse(req.url).pathname; @@ -64,7 +36,8 @@ function reactRouter(Router, options) { return; } res.statusCode = statusCode; - res.end('' + doctype + rendered); + res.setHeader('Content-Type', meta.contentType); + res.end('' + meta.doctype + rendered); }; try { From bbb266f0fba6318061e1412accffddd48b3f95b7 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 16:26:59 -0400 Subject: [PATCH 09/14] Linting fixes Also adds "eqnull: true" to the linting config to allow us to treat `null` and `undefined` the same way. --- .jshintrc | 1 + lib/Router.js | 1 - lib/RoutingEnvironmentMixin.js | 1 - lib/contrib/connect.js | 4 ++-- lib/getContentMeta.js | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.jshintrc b/.jshintrc index 48ddab3..b045d22 100644 --- a/.jshintrc +++ b/.jshintrc @@ -4,6 +4,7 @@ "asi": true, "expr": true, "globalstrict": true, + "eqnull": true, "globals": { "window": false, "Buffer": false, diff --git a/lib/Router.js b/lib/Router.js index be16599..0debcce 100644 --- a/lib/Router.js +++ b/lib/Router.js @@ -1,7 +1,6 @@ "use strict"; var React = require('react'); -var merge = require('react/lib/merge'); var Preloaded = require('react-async/lib/Preloaded'); var RoutingEnvironmentMixin = require('./RoutingEnvironmentMixin'); var matchRoutes = require('./matchRoutes'); diff --git a/lib/RoutingEnvironmentMixin.js b/lib/RoutingEnvironmentMixin.js index 67bbf3f..aa92e24 100644 --- a/lib/RoutingEnvironmentMixin.js +++ b/lib/RoutingEnvironmentMixin.js @@ -3,7 +3,6 @@ var React = require('react'); var merge = require('react/lib/merge'); var Environment = require('./environment'); -var invariant = require('react/lib/invariant'); /** * Mixin which makes router bound to an environment. This is the glue that binds diff --git a/lib/contrib/connect.js b/lib/contrib/connect.js index 54e9c11..bc25718 100644 --- a/lib/contrib/connect.js +++ b/lib/contrib/connect.js @@ -1,6 +1,6 @@ "use strict"; -var URL = require('url'); +var parseUrl = require('url').parse; var React = require('react'); var merge = require('react/lib/merge'); var invariant = require('react/lib/invariant'); @@ -21,7 +21,7 @@ function reactRouter(Router, options) { options && options.contentType); var props = options && options.props || {}; return function(req, res, next) { - var pathname = URL.parse(req.url).pathname; + var pathname = parseUrl(req.url).pathname; if (pathname == null) { next(new Error('Invalid URL: ' + req.url)); return; diff --git a/lib/getContentMeta.js b/lib/getContentMeta.js index f923272..dcd8405 100644 --- a/lib/getContentMeta.js +++ b/lib/getContentMeta.js @@ -20,7 +20,7 @@ var doctypes = { function getContentMeta(doctype, contentType) { - var ct, dt, contentTypesToDoctypes, doctypesToShortcuts, shortcuts; + var ct, dt, doctypesToShortcuts, shortcuts; if (doctype == null && contentType == null) { doctype = 'default'; From 72fffb17f81b42bf60dedf1d7b2dc9f89620b8de Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 16:38:26 -0400 Subject: [PATCH 10/14] Rename module; only export middleware --- lib/contrib/{connect.js => connect-middleware.js} | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename lib/contrib/{connect.js => connect-middleware.js} (97%) diff --git a/lib/contrib/connect.js b/lib/contrib/connect-middleware.js similarity index 97% rename from lib/contrib/connect.js rename to lib/contrib/connect-middleware.js index bc25718..dfddcaf 100644 --- a/lib/contrib/connect.js +++ b/lib/contrib/connect-middleware.js @@ -57,6 +57,4 @@ function reactRouter(Router, options) { }; } -module.exports = { - reactRouter: reactRouter -}; +module.exports = reactRouter; From df701b236694518c1c5ffec8448ee3446cbf4401 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 21:07:17 -0400 Subject: [PATCH 11/14] Lowercase "html" in html5 doctype It doesn't matter, but this is the form used in the spec docs. (Only HTML4 capitalizes it.) --- lib/getContentMeta.js | 2 +- tests/getContentMeta.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/getContentMeta.js b/lib/getContentMeta.js index dcd8405..92e3687 100644 --- a/lib/getContentMeta.js +++ b/lib/getContentMeta.js @@ -2,7 +2,7 @@ var doctypes = { 'text/html': { - '': ['default', '5', 'html', 'html5'], + '': ['default', '5', 'html', 'html5'], '': ['4', 'html4'] }, 'application/xhtml+xml': { diff --git a/tests/getContentMeta.js b/tests/getContentMeta.js index abbdc93..f31d361 100644 --- a/tests/getContentMeta.js +++ b/tests/getContentMeta.js @@ -6,7 +6,7 @@ describe('getContentMeta', function() { it('finds doctype by content type', function() { assert.strictEqual( getContentMeta(null, 'text/html').doctype, - '' + '' ); assert.strictEqual( getContentMeta(null, 'application/xhtml+xml').doctype, @@ -81,14 +81,14 @@ describe('getContentMeta', function() { it('defaults to html5', function() { assert.deepEqual( getContentMeta(), - {doctype: '', contentType: 'text/html'} + {doctype: '', contentType: 'text/html'} ); }); it('defaults to html5', function() { assert.deepEqual( getContentMeta(), - {doctype: '', contentType: 'text/html'} + {doctype: '', contentType: 'text/html'} ); }); From fddd7fcc12bf13c5846e860d922bad4029c4d98e Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Tue, 15 Apr 2014 21:12:13 -0400 Subject: [PATCH 12/14] Ensure content type can be overridden (even with short doctype) --- tests/getContentMeta.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/getContentMeta.js b/tests/getContentMeta.js index f31d361..2e3f510 100644 --- a/tests/getContentMeta.js +++ b/tests/getContentMeta.js @@ -76,6 +76,13 @@ describe('getContentMeta', function() { getContentMeta(4, 'text/html').doctype, '' ); + assert.deepEqual( + getContentMeta('strict', 'text/html'), + { + doctype: '', + contentType: 'text/html' + } + ); }); it('defaults to html5', function() { From 91116a2b2c364e03b76362f6ecfd66ae3f76bc35 Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Thu, 24 Apr 2014 11:57:36 -0400 Subject: [PATCH 13/14] Use ReactAsync to render --- lib/contrib/connect-middleware.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/contrib/connect-middleware.js b/lib/contrib/connect-middleware.js index dfddcaf..6951237 100644 --- a/lib/contrib/connect-middleware.js +++ b/lib/contrib/connect-middleware.js @@ -2,6 +2,7 @@ var parseUrl = require('url').parse; var React = require('react'); +var ReactAsync = require('react-async'); var merge = require('react/lib/merge'); var invariant = require('react/lib/invariant'); var getContentMeta = require('../getContentMeta'); @@ -49,8 +50,18 @@ function reactRouter(Router, options) { callback(); } })); - rendered = React.renderComponentToString(app); - callback(); + ReactAsync.renderComponentToStringWithAsyncState( + app, + function(err, markup) { + if (err) { + next(err); + } else { + rendered = markup; + callback(); + } + + } + ); } catch (err) { next(err); } From 5750bec3b46217c2e0e5be6bee7c152fe0b0351d Mon Sep 17 00:00:00 2001 From: Matthew Dapena-Tretter Date: Thu, 24 Apr 2014 12:23:00 -0400 Subject: [PATCH 14/14] Clean up callback stuff a bit --- lib/contrib/connect-middleware.js | 47 ++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/contrib/connect-middleware.js b/lib/contrib/connect-middleware.js index 6951237..8b38bf9 100644 --- a/lib/contrib/connect-middleware.js +++ b/lib/contrib/connect-middleware.js @@ -28,38 +28,53 @@ function reactRouter(Router, options) { return; } - var statusCode; - var rendered; + // This callback is meant to be called multiple times with whatever + // information is available. It will aggregate the information and send a + // response or emit an error as soon as is possible. + var memo = {}; + var callback = function(err, statusCode, markup) { + if (memo.complete) { + return; // Don't take action more than once. + } + if (err != null) { + // If we errored, we're done. + memo.complete = true; + next(err); + return; + } + + // Remember the status code and markup (since we may not get them at the + // same time). + if (statusCode != null) { + memo.statusCode = statusCode; + } + if (markup != null) { + memo.markup = markup; + } - var callback = function() { // Wait for the render to complete and onDispatch to be invoked. - if (rendered == null || statusCode == null) { + if (memo.markup == null || memo.statusCode == null) { return; } - res.statusCode = statusCode; + memo.complete = true; + res.statusCode = memo.statusCode; res.setHeader('Content-Type', meta.contentType); - res.end('' + meta.doctype + rendered); + res.end('' + meta.doctype + memo.markup); }; try { var app = Router(merge(props, { path: pathname, onDispatch: function(path, navigation) { - statusCode = (navigation && navigation.match && - navigation.match.isNotFound ? 404 : 200); - callback(); + var statusCode = (navigation && navigation.match && + navigation.match.isNotFound ? 404 : 200); + callback(null, statusCode); } })); ReactAsync.renderComponentToStringWithAsyncState( app, function(err, markup) { - if (err) { - next(err); - } else { - rendered = markup; - callback(); - } - + callback(err, null, markup); } ); } catch (err) {