From c5202aa072c5d563f55fe924ad16f0a14edacc26 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Thu, 11 Aug 2016 15:10:36 -0700 Subject: [PATCH 1/7] tagNames on root containers now working --- .../react-server/core/ClientController.js | 16 +++++++++------ .../core/components/RootContainer.js | 6 ++++-- .../core/components/RootElement.js | 2 +- .../react-server/core/renderMiddleware.js | 20 ++++++++++++++----- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index dd0c71529..009576e6c 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -580,10 +580,10 @@ class ClientController extends EventEmitter { // if (this._reuseDom) { oldRootElement = document.querySelector( - `div[${REACT_SERVER_DATA_ATTRIBUTE}="${index}"]` + `*[${REACT_SERVER_DATA_ATTRIBUTE}="${index}"]` ); oldRootContainer = document.querySelector( - `div[${PAGE_CONTAINER_NODE_ID}="${index}"]` + `*[${PAGE_CONTAINER_NODE_ID}="${index}"]` ); } @@ -650,7 +650,11 @@ class ClientController extends EventEmitter { _.forEach( getRootElementAttributes(element), - (v, k) => root.setAttribute(k, v) + (v, k) => { + if (k !== 'tagName') { + root.setAttribute(k, v) + } + } ); totalRenderTime += timer.stop(); @@ -724,7 +728,7 @@ class ClientController extends EventEmitter { logger.debug("Removing previous page's React components"); [].slice.call( - document.querySelectorAll(`div[${REACT_SERVER_DATA_ATTRIBUTE}]`) + document.querySelectorAll(`*[${REACT_SERVER_DATA_ATTRIBUTE}]`) ).forEach((root, i) => { if (i >= index) { // Since this node has a "data-react-server-root-id" @@ -738,7 +742,7 @@ class ClientController extends EventEmitter { }); [].slice.call( - document.querySelectorAll(`div[${PAGE_CONTAINER_NODE_ID}]`) + document.querySelectorAll(`*[${PAGE_CONTAINER_NODE_ID}]`) ).forEach((root, i) => { if (i >= index) { // Gotta get rid of our containers, @@ -860,7 +864,7 @@ class ClientController extends EventEmitter { for (var i = startIndex; i <= endIndex; i++) { this._ensureRootNodeDfd(i).resolve( this.mountNode.querySelector( - `div[${REACT_SERVER_DATA_ATTRIBUTE}="${i}"]` + `*[${REACT_SERVER_DATA_ATTRIBUTE}="${i}"]` ) ); } diff --git a/packages/react-server/core/components/RootContainer.js b/packages/react-server/core/components/RootContainer.js index 004112ec2..aa4201775 100644 --- a/packages/react-server/core/components/RootContainer.js +++ b/packages/react-server/core/components/RootContainer.js @@ -23,9 +23,11 @@ RootContainer.defaultProps = { } RootContainer.flattenForRender = function(element) { - return [{containerOpen: getRootElementAttributes(element)}] + let attrs = getRootElementAttributes(element) || {}; + attrs.tagName = attrs.tagName || 'div'; + return [{containerOpen: true, attrs: attrs}] .concat(prepChildren(element)) - .concat([{containerClose: true}]) + .concat([{containerClose: true, attrs: attrs}]) .reduce((m,v) => m.concat(Array.isArray(v)?v:[v]), []) } diff --git a/packages/react-server/core/components/RootElement.js b/packages/react-server/core/components/RootElement.js index 7c8f21c71..883e8e268 100644 --- a/packages/react-server/core/components/RootElement.js +++ b/packages/react-server/core/components/RootElement.js @@ -103,7 +103,7 @@ RootElement.getRootElementAttributes = function(element) { 'id', 'style', ].forEach(k => props[k] && (attrs[k] = props[k])); - + attrs.tagName = props.tagName || 'div'; return attrs; } diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index 58b64ae6b..71386b34f 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -866,26 +866,36 @@ function writeElement(res, element, i){ html : '', } } + + console.log(element) + + let tagName = 'div'; + if (element.attrs.tagName) { + tagName = element.attrs.tagName; + delete element.attrs.tagName; + } + if (element.containerOpen) { - res.write(`
` ${k}="${attrfy(v)}"`) + res.write(`<${tagName} ${PAGE_CONTAINER_NODE_ID}=${i}${ + _.map(element.attrs, (v, k) => ` ${k}="${attrfy(v)}"`) }>`); } else if (element.containerClose) { - res.write('
'); + res.write(``); } else if (element.isTheFold) { // Okay, we've sent all of our above-the-fold HTML, // now we can let the client start waking nodes up. bootstrapClient(res, i) } else { - res.write(`
` ${k}="${attrfy(v)}"`) - }>${element.html}
`); + }>${element.html}`); } } From 0fbc64bc99dde4f23233c6bbfecb188dc4eb2b83 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Thu, 11 Aug 2016 15:10:36 -0700 Subject: [PATCH 2/7] tagNames on root containers now working --- packages/react-server/core/ClientController.js | 16 ++++++++++------ .../core/components/RootContainer.js | 6 ++++-- .../core/components/RootElement.js | 2 +- packages/react-server/core/renderMiddleware.js | 18 +++++++++++++----- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index dd0c71529..009576e6c 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -580,10 +580,10 @@ class ClientController extends EventEmitter { // if (this._reuseDom) { oldRootElement = document.querySelector( - `div[${REACT_SERVER_DATA_ATTRIBUTE}="${index}"]` + `*[${REACT_SERVER_DATA_ATTRIBUTE}="${index}"]` ); oldRootContainer = document.querySelector( - `div[${PAGE_CONTAINER_NODE_ID}="${index}"]` + `*[${PAGE_CONTAINER_NODE_ID}="${index}"]` ); } @@ -650,7 +650,11 @@ class ClientController extends EventEmitter { _.forEach( getRootElementAttributes(element), - (v, k) => root.setAttribute(k, v) + (v, k) => { + if (k !== 'tagName') { + root.setAttribute(k, v) + } + } ); totalRenderTime += timer.stop(); @@ -724,7 +728,7 @@ class ClientController extends EventEmitter { logger.debug("Removing previous page's React components"); [].slice.call( - document.querySelectorAll(`div[${REACT_SERVER_DATA_ATTRIBUTE}]`) + document.querySelectorAll(`*[${REACT_SERVER_DATA_ATTRIBUTE}]`) ).forEach((root, i) => { if (i >= index) { // Since this node has a "data-react-server-root-id" @@ -738,7 +742,7 @@ class ClientController extends EventEmitter { }); [].slice.call( - document.querySelectorAll(`div[${PAGE_CONTAINER_NODE_ID}]`) + document.querySelectorAll(`*[${PAGE_CONTAINER_NODE_ID}]`) ).forEach((root, i) => { if (i >= index) { // Gotta get rid of our containers, @@ -860,7 +864,7 @@ class ClientController extends EventEmitter { for (var i = startIndex; i <= endIndex; i++) { this._ensureRootNodeDfd(i).resolve( this.mountNode.querySelector( - `div[${REACT_SERVER_DATA_ATTRIBUTE}="${i}"]` + `*[${REACT_SERVER_DATA_ATTRIBUTE}="${i}"]` ) ); } diff --git a/packages/react-server/core/components/RootContainer.js b/packages/react-server/core/components/RootContainer.js index 004112ec2..aa4201775 100644 --- a/packages/react-server/core/components/RootContainer.js +++ b/packages/react-server/core/components/RootContainer.js @@ -23,9 +23,11 @@ RootContainer.defaultProps = { } RootContainer.flattenForRender = function(element) { - return [{containerOpen: getRootElementAttributes(element)}] + let attrs = getRootElementAttributes(element) || {}; + attrs.tagName = attrs.tagName || 'div'; + return [{containerOpen: true, attrs: attrs}] .concat(prepChildren(element)) - .concat([{containerClose: true}]) + .concat([{containerClose: true, attrs: attrs}]) .reduce((m,v) => m.concat(Array.isArray(v)?v:[v]), []) } diff --git a/packages/react-server/core/components/RootElement.js b/packages/react-server/core/components/RootElement.js index 7c8f21c71..883e8e268 100644 --- a/packages/react-server/core/components/RootElement.js +++ b/packages/react-server/core/components/RootElement.js @@ -103,7 +103,7 @@ RootElement.getRootElementAttributes = function(element) { 'id', 'style', ].forEach(k => props[k] && (attrs[k] = props[k])); - + attrs.tagName = props.tagName || 'div'; return attrs; } diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index 58b64ae6b..d457f9799 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -866,26 +866,34 @@ function writeElement(res, element, i){ html : '', } } + + let tagName = 'div'; + if (element.attrs.tagName) { + tagName = element.attrs.tagName; + delete element.attrs.tagName; + } + if (element.containerOpen) { - res.write(`
` ${k}="${attrfy(v)}"`) + res.write(`<${tagName} ${PAGE_CONTAINER_NODE_ID}=${i}${ + _.map(element.attrs, (v, k) => ` ${k}="${attrfy(v)}"`) }>`); } else if (element.containerClose) { - res.write('
'); + res.write(``); } else if (element.isTheFold) { // Okay, we've sent all of our above-the-fold HTML, // now we can let the client start waking nodes up. bootstrapClient(res, i) } else { - res.write(`
` ${k}="${attrfy(v)}"`) - }>${element.html}
`); + }>${element.html}`); } } From 9a6d065c9dba5e44c9d5a0c3f018433169b3efc9 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Sat, 13 Aug 2016 09:43:37 -0700 Subject: [PATCH 3/7] updates to attributes test page --- .../pages/root/attributes.js | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/react-server-test-pages/pages/root/attributes.js b/packages/react-server-test-pages/pages/root/attributes.js index c0a6c6b0e..3437510e6 100644 --- a/packages/react-server-test-pages/pages/root/attributes.js +++ b/packages/react-server-test-pages/pages/root/attributes.js @@ -1,8 +1,9 @@ import {RootContainer, RootElement, Link} from "react-server"; +require("./attributes.css"); const url = color => `/root/attributes?color=${color}`; -const Links = opts => +const ColorLinks = opts => Red | Yellow | Green @@ -13,16 +14,33 @@ export default class RootAttributesPage { const color = this.getRequest().getQuery().color || "white"; const style = `background-color: ${color}`; return -
Background below the hr should be {color}
-
(normal)
-
(reuseDom)
-
+ +

style

+
(normal)
+
(reuseDom)
-
RootContainer
+
RootContainer should be {color}
 -
RootElement
+
RootElement should be {color}
+ + +

className

+ +
RootContainer should be "salmon"
+ + +
RootElement should be "salmon"
 + +

tagName

+ +
RootContainer should be "turquoise"
+ + +
RootElement should be "turquoise"
+ + } } From 99b7227cab89db1c9f5ea1ec6202e82253314b10 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Sat, 13 Aug 2016 09:56:15 -0700 Subject: [PATCH 4/7] Revert "updates to attributes test page" This reverts commit 9a6d065c9dba5e44c9d5a0c3f018433169b3efc9. --- .../pages/root/attributes.js | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/packages/react-server-test-pages/pages/root/attributes.js b/packages/react-server-test-pages/pages/root/attributes.js index 3437510e6..c0a6c6b0e 100644 --- a/packages/react-server-test-pages/pages/root/attributes.js +++ b/packages/react-server-test-pages/pages/root/attributes.js @@ -1,9 +1,8 @@ import {RootContainer, RootElement, Link} from "react-server"; -require("./attributes.css"); const url = color => `/root/attributes?color=${color}`; -const ColorLinks = opts => +const Links = opts => Red | Yellow | Green @@ -14,33 +13,16 @@ export default class RootAttributesPage { const color = this.getRequest().getQuery().color || "white"; const style = `background-color: ${color}`; return - -

style

-
(normal)
-
(reuseDom)
+
Background below the hr should be {color}
+
(normal)
+
(reuseDom)
+
-
RootContainer should be {color}
+
RootContainer
 -
RootElement should be {color}
- - -

className

- -
RootContainer should be "salmon"
- - -
RootElement should be "salmon"
+
RootElement
 - -

tagName

- -
RootContainer should be "turquoise"
- - -
RootElement should be "turquoise"
- - } } From b09a5dc01c7a3ab144be932cd6de6dbf97fdfe2f Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Sat, 13 Aug 2016 09:57:09 -0700 Subject: [PATCH 5/7] updates to attributes test page --- .../pages/root/attributes.css | 26 +++++++++++++++ .../pages/root/attributes.js | 32 +++++++++++++++---- 2 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 packages/react-server-test-pages/pages/root/attributes.css diff --git a/packages/react-server-test-pages/pages/root/attributes.css b/packages/react-server-test-pages/pages/root/attributes.css new file mode 100644 index 000000000..8b4c4a38b --- /dev/null +++ b/packages/react-server-test-pages/pages/root/attributes.css @@ -0,0 +1,26 @@ +body { + font-family: Helvetica, sans-serif; +} + +h2 { + border-bottom: 1px solid #ccc; + padding-bottom: .2rem; +} + +*[data-react-server-container], +*[data-react-server-root-id] { + max-width: 600px; +} + +.example { + padding: .3rem; + margin-top: .5rem; +} + +.example-class { + background-color: salmon; +} + +section { + background-color: turquoise; +} \ No newline at end of file diff --git a/packages/react-server-test-pages/pages/root/attributes.js b/packages/react-server-test-pages/pages/root/attributes.js index c0a6c6b0e..2bf8e5322 100644 --- a/packages/react-server-test-pages/pages/root/attributes.js +++ b/packages/react-server-test-pages/pages/root/attributes.js @@ -1,8 +1,9 @@ import {RootContainer, RootElement, Link} from "react-server"; +require("./attributes.css"); const url = color => `/root/attributes?color=${color}`; -const Links = opts => +const ColorLinks = opts => Red | Yellow | Green @@ -13,16 +14,33 @@ export default class RootAttributesPage { const color = this.getRequest().getQuery().color || "white"; const style = `background-color: ${color}`; return -
Background below the hr should be {color}
-
(normal)
-
(reuseDom)
-
+ +

style

+
(normal)
+
(reuseDom)
-
RootContainer
+
RootContainer should be {color}
 -
RootElement
+
RootElement should be {color}
+ + +

className

+ +
RootContainer should be "salmon" have the css class "example-class" applied when inspected
+ + +
RootElement should be "salmon" have the css class "example-class" applied when inspected
 + +

tagName

+ +
RootContainer should be "turquoise" and have the node type "section" when inspected
+ + +
RootElement should be "turquoise" and have the node type "section" when inspected
+ + } } From 8d362c1c976863806930d4d551dc803fd0b05e70 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Sat, 13 Aug 2016 10:30:50 -0700 Subject: [PATCH 6/7] moving tagName out of attrs --- packages/react-server/core/ClientController.js | 6 +----- .../core/components/RootContainer.js | 12 ++++++++---- .../core/components/RootElement.js | 5 ++++- packages/react-server/core/renderMiddleware.js | 18 ++++++++++-------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index 009576e6c..7bd4f9f8e 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -650,11 +650,7 @@ class ClientController extends EventEmitter { _.forEach( getRootElementAttributes(element), - (v, k) => { - if (k !== 'tagName') { - root.setAttribute(k, v) - } - } + (v, k) => {root.setAttribute(k, v)} ); totalRenderTime += timer.stop(); diff --git a/packages/react-server/core/components/RootContainer.js b/packages/react-server/core/components/RootContainer.js index aa4201775..7a299319a 100644 --- a/packages/react-server/core/components/RootContainer.js +++ b/packages/react-server/core/components/RootContainer.js @@ -23,11 +23,15 @@ RootContainer.defaultProps = { } RootContainer.flattenForRender = function(element) { - let attrs = getRootElementAttributes(element) || {}; - attrs.tagName = attrs.tagName || 'div'; - return [{containerOpen: true, attrs: attrs}] + let tagName = element.props.tagName || 'div'; + return [{containerOpen: true, + attrs: getRootElementAttributes(element), + tagName: tagName, + }] .concat(prepChildren(element)) - .concat([{containerClose: true, attrs: attrs}]) + .concat([{containerClose: true, + tagName: tagName, + }]) .reduce((m,v) => m.concat(Array.isArray(v)?v:[v]), []) } diff --git a/packages/react-server/core/components/RootElement.js b/packages/react-server/core/components/RootElement.js index 883e8e268..0caa2ff87 100644 --- a/packages/react-server/core/components/RootElement.js +++ b/packages/react-server/core/components/RootElement.js @@ -103,10 +103,13 @@ RootElement.getRootElementAttributes = function(element) { 'id', 'style', ].forEach(k => props[k] && (attrs[k] = props[k])); - attrs.tagName = props.tagName || 'div'; return attrs; } +RootElement.getRootElementTagName = function(element) { + return element.props.tagName || 'div'; +} + RootElement.ensureRootElementWithContainer = function(element, container) { // If it's _already_ a root element (or the fold), pass it along. diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index d457f9799..19efeb949 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -15,7 +15,7 @@ var logger = require('./logging').getLogger(__LOGGER__), PageUtil = require('./util/PageUtil'), ReactServerAgent = require('./ReactServerAgent'), StringEscapeUtil = require('./util/StringEscapeUtil'), - {getRootElementAttributes} = require('./components/RootElement'), + {getRootElementAttributes, getRootElementTagName} = require('./components/RootElement'), {PAGE_CSS_NODE_ID, PAGE_LINK_NODE_ID, PAGE_CONTENT_NODE_ID, PAGE_CONTAINER_NODE_ID} = require('./constants'); var _ = { @@ -786,6 +786,7 @@ function renderElement(res, element, context) { , timer = logger.timer(`renderElement.individual.${name}`) , html = '' , attrs = {} + , tagName = 'div' try { if (element !== null) { @@ -793,6 +794,7 @@ function renderElement(res, element, context) { React.cloneElement(element, { context: context }) ); attrs = getRootElementAttributes(element); + tagName = getRootElementTagName(element); } } catch (err) { // A component failing to render is not fatal. We've already @@ -814,7 +816,7 @@ function renderElement(res, element, context) { RLS().renderTime || (RLS().renderTime = 0); RLS().renderTime += individualTime; - return { html, attrs }; + return { html, attrs, tagName }; } // Write as many elements out in a row as possible and then flush output. @@ -866,12 +868,12 @@ function writeElement(res, element, i){ html : '', } } - - let tagName = 'div'; - if (element.attrs.tagName) { - tagName = element.attrs.tagName; - delete element.attrs.tagName; - } + let tagName = element.tagName || 'div'; + console.log('el', element) + // if (element.attrs.tagName) { + // tagName = element.attrs.tagName; + // delete element.attrs.tagName; + // } if (element.containerOpen) { res.write(`<${tagName} ${PAGE_CONTAINER_NODE_ID}=${i}${ From 0d91e5c9f093e51ed11dc85b8ef355dbd82ec600 Mon Sep 17 00:00:00 2001 From: Kevin Jonson Date: Sat, 13 Aug 2016 11:05:29 -0700 Subject: [PATCH 7/7] further work, but not finished yet --- .../react-server/core/ClientController.js | 19 ++++++++++++------- .../core/components/RootElement.js | 5 ++--- .../react-server/core/renderMiddleware.js | 5 ----- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index 7bd4f9f8e..db3a12bd9 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -599,7 +599,7 @@ class ClientController extends EventEmitter { mountNode = oldRootContainer; this._updateContainerNodeAttributes( mountNode, - element.containerOpen + element.attrs ); } else if (this._reuseDom && element.containerClose && !oldRootContainer && !oldRootElement) { mountNode = mountNode.parentNode; @@ -613,7 +613,8 @@ class ClientController extends EventEmitter { // our new mountNode. mountNode = this._createContainerNode( mountNode, - element.containerOpen, + element.tagName, + element.attrs, index ); } else if (element.containerClose) { @@ -625,7 +626,11 @@ class ClientController extends EventEmitter { // Need a new root element in our // current mountNode. - root = this._createReactServerRootNode(mountNode, index) + root = this._createReactServerRootNode( + mountNode, + element.tagName, + index + ); } } @@ -754,15 +759,15 @@ class ClientController extends EventEmitter { /** * This method creates a new div to render a ReactElement in to at the end of the mount node. */ - _createReactServerRootNode(mountNode, index) { - var root = document.createElement("div"); + _createReactServerRootNode(mountNode, tagName, index) { + var root = document.createElement(tagName || 'div'); root.setAttribute(REACT_SERVER_DATA_ATTRIBUTE, index); mountNode.appendChild(root); return root; } - _createContainerNode(mountNode, attrs, i) { - var node = document.createElement("div"); + _createContainerNode(mountNode, tagName, attrs, i) { + var node = document.createElement(tagName || 'div'); node.setAttribute(PAGE_CONTAINER_NODE_ID, i); _.forEach(attrs, (v, k) => node.setAttribute(k, v)); mountNode.appendChild(node); diff --git a/packages/react-server/core/components/RootElement.js b/packages/react-server/core/components/RootElement.js index 0caa2ff87..4e3cb1559 100644 --- a/packages/react-server/core/components/RootElement.js +++ b/packages/react-server/core/components/RootElement.js @@ -125,9 +125,8 @@ RootElement.ensureRootElementWithContainer = function(element, container) { return element; } - const {listen, when} = container.props; - - return {element}; + const {listen, when, tagName} = container.props; + return {element}; } RootElement.ensureRootElement = function(element){ diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index 19efeb949..034b4758d 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -869,11 +869,6 @@ function writeElement(res, element, i){ } } let tagName = element.tagName || 'div'; - console.log('el', element) - // if (element.attrs.tagName) { - // tagName = element.attrs.tagName; - // delete element.attrs.tagName; - // } if (element.containerOpen) { res.write(`<${tagName} ${PAGE_CONTAINER_NODE_ID}=${i}${