Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't warn about casing in SSR for non-HTML NS #10452

Merged
merged 1 commit into from
Aug 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 3 additions & 30 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ var CHILDREN = 'children';
var STYLE = 'style';
var HTML = '__html';

var {
html: HTML_NAMESPACE,
svg: SVG_NAMESPACE,
mathml: MATH_NAMESPACE,
} = DOMNamespaces;
var {Namespaces: {html: HTML_NAMESPACE}, getIntrinsicNamespace} = DOMNamespaces;

if (__DEV__) {
var warnedUnknownTags = {
Expand Down Expand Up @@ -295,32 +291,7 @@ function updateDOMProperties(
}
}

// Assumes there is no parent namespace.
function getIntrinsicNamespace(type: string): string {
switch (type) {
case 'svg':
return SVG_NAMESPACE;
case 'math':
return MATH_NAMESPACE;
default:
return HTML_NAMESPACE;
}
}

var ReactDOMFiberComponent = {
getChildNamespace(parentNamespace: string | null, type: string): string {
if (parentNamespace == null || parentNamespace === HTML_NAMESPACE) {
// No (or default) parent namespace: potential entry point.
return getIntrinsicNamespace(type);
}
if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') {
// We're leaving SVG.
return HTML_NAMESPACE;
}
// By default, pass namespace below.
return parentNamespace;
},

createElement(
type: *,
props: Object,
Expand All @@ -343,6 +314,8 @@ var ReactDOMFiberComponent = {
}
if (namespaceURI === HTML_NAMESPACE) {
if (__DEV__) {
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - probably meant "<MATH>" not "<mATH>"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I meant. :)

warning(
isCustomComponentTag || type === type.toLowerCase(),
'<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
Expand Down
3 changes: 2 additions & 1 deletion src/renderers/dom/fiber/ReactDOMFiberEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {Fiber} from 'ReactFiber';
import type {ReactNodeList} from 'ReactTypes';

require('checkReact');
var DOMNamespaces = require('DOMNamespaces');
var ExecutionEnvironment = require('fbjs/lib/ExecutionEnvironment');
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
var ReactControlledComponent = require('ReactControlledComponent');
Expand Down Expand Up @@ -44,9 +45,9 @@ var {ROOT_ATTRIBUTE_NAME} = require('DOMProperty');
var findDOMNode = require('findDOMNode');
var invariant = require('fbjs/lib/invariant');

var {getChildNamespace} = DOMNamespaces;
var {
createElement,
getChildNamespace,
setInitialProperties,
diffProperties,
updateProperties,
Expand Down
44 changes: 39 additions & 5 deletions src/renderers/dom/shared/DOMNamespaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,44 @@

'use strict';

var DOMNamespaces = {
html: 'http://www.w3.org/1999/xhtml',
mathml: 'http://www.w3.org/1998/Math/MathML',
svg: 'http://www.w3.org/2000/svg',
const HTML_NAMESPACE = 'http://www.w3.org/1999/xhtml';
const MATH_NAMESPACE = 'http://www.w3.org/1998/Math/MathML';
const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';

const Namespaces = {
html: HTML_NAMESPACE,
mathml: MATH_NAMESPACE,
svg: SVG_NAMESPACE,
};

module.exports = DOMNamespaces;
// Assumes there is no parent namespace.
function getIntrinsicNamespace(type: string): string {
switch (type) {
case 'svg':
return SVG_NAMESPACE;
case 'math':
return MATH_NAMESPACE;
default:
return HTML_NAMESPACE;
}
}

function getChildNamespace(
parentNamespace: string | null,
type: string,
): string {
if (parentNamespace == null || parentNamespace === HTML_NAMESPACE) {
// No (or default) parent namespace: potential entry point.
return getIntrinsicNamespace(type);
}
if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') {
// We're leaving SVG.
return HTML_NAMESPACE;
}
// By default, pass namespace below.
return parentNamespace;
}

exports.Namespaces = Namespaces;
exports.getIntrinsicNamespace = getIntrinsicNamespace;
exports.getChildNamespace = getChildNamespace;
32 changes: 32 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -772,4 +772,36 @@ describe('ReactDOMServer', () => {
);
}).toThrowError(/Cannot assign to read only property.*/);
});

it('warns about lowercase html but not in svg tags', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome - thanks for adding a test!

spyOn(console, 'error');
function CompositeG(props) {
// Make sure namespace passes through composites
return <g>{props.children}</g>;
}
ReactDOMServer.renderToStaticMarkup(
<div>
<inPUT />
<svg>
<CompositeG>
<linearGradient />
<foreignObject>
{/* back to HTML */}
<iFrame />
</foreignObject>
</CompositeG>
</svg>
</div>,
);
expect(console.error.calls.count()).toBe(2);
expect(console.error.calls.argsFor(0)[0]).toBe(
'Warning: <inPUT /> is using uppercase HTML. Always use lowercase ' +
'HTML tags in React.',
);
// linearGradient doesn't warn
expect(console.error.calls.argsFor(1)[0]).toBe(
'Warning: <iFrame /> is using uppercase HTML. Always use lowercase ' +
'HTML tags in React.',
);
});
});
4 changes: 2 additions & 2 deletions src/renderers/dom/shared/setInnerHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

'use strict';

var DOMNamespaces = require('DOMNamespaces');
var Namespaces = require('DOMNamespaces').Namespaces;
var createMicrosoftUnsafeLocalFunction = require('createMicrosoftUnsafeLocalFunction');

// SVG temp container for IE lacking innerHTML
Expand All @@ -28,7 +28,7 @@ var setInnerHTML = createMicrosoftUnsafeLocalFunction(function(node, html) {
// IE does not have innerHTML for SVG nodes, so instead we inject the
// new markup in a temp node and then move the child nodes across into
// the target node
if (node.namespaceURI === DOMNamespaces.svg && !('innerHTML' in node)) {
if (node.namespaceURI === Namespaces.svg && !('innerHTML' in node)) {
reusableSVGContainer =
reusableSVGContainer || document.createElement('div');
reusableSVGContainer.innerHTML = '<svg>' + html + '</svg>';
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

// TODO: can we express this test with only public API?
var setInnerHTML = require('setInnerHTML');
var DOMNamespaces = require('DOMNamespaces');
var Namespaces = require('DOMNamespaces').Namespaces;

describe('setInnerHTML', () => {
describe('when the node has innerHTML property', () => {
Expand All @@ -31,7 +31,7 @@ describe('setInnerHTML', () => {
xit('sets innerHTML on it', () => {
// Create a mock node that looks like an SVG in IE (without innerHTML)
var node = {
namespaceURI: DOMNamespaces.svg,
namespaceURI: Namespaces.svg,
appendChild: jasmine.createSpy(),
};

Expand Down
4 changes: 2 additions & 2 deletions src/renderers/dom/stack/client/DOMLazyTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

'use strict';

var DOMNamespaces = require('DOMNamespaces');
var Namespaces = require('DOMNamespaces').Namespaces;
var setInnerHTML = require('setInnerHTML');
var {DOCUMENT_FRAGMENT_NODE, ELEMENT_NODE} = require('HTMLNodeType');
var createMicrosoftUnsafeLocalFunction = require('createMicrosoftUnsafeLocalFunction');
Expand Down Expand Up @@ -68,7 +68,7 @@ var insertTreeBefore = createMicrosoftUnsafeLocalFunction(function(
(tree.node.nodeType === ELEMENT_NODE &&
tree.node.nodeName.toLowerCase() === 'object' &&
(tree.node.namespaceURI == null ||
tree.node.namespaceURI === DOMNamespaces.html))
tree.node.namespaceURI === Namespaces.html))
) {
insertTreeChildren(tree);
parentNode.insertBefore(tree.node, referenceNode);
Expand Down
16 changes: 8 additions & 8 deletions src/renderers/dom/stack/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
var AutoFocusUtils = require('AutoFocusUtils');
var CSSPropertyOperations = require('CSSPropertyOperations');
var DOMLazyTree = require('DOMLazyTree');
var DOMNamespaces = require('DOMNamespaces');
var Namespaces = require('DOMNamespaces').Namespaces;
var DOMMarkupOperations = require('DOMMarkupOperations');
var DOMProperty = require('DOMProperty');
var DOMPropertyOperations = require('DOMPropertyOperations');
Expand Down Expand Up @@ -512,11 +512,11 @@ ReactDOMComponent.Mixin = {
}
if (
namespaceURI == null ||
(namespaceURI === DOMNamespaces.svg && parentTag === 'foreignobject')
(namespaceURI === Namespaces.svg && parentTag === 'foreignobject')
) {
namespaceURI = DOMNamespaces.html;
namespaceURI = Namespaces.html;
}
if (namespaceURI === DOMNamespaces.html) {
if (namespaceURI === Namespaces.html) {
if (__DEV__) {
warning(
isCustomComponentTag || this._tag === this._currentElement.type,
Expand All @@ -526,9 +526,9 @@ ReactDOMComponent.Mixin = {
);
}
if (this._tag === 'svg') {
namespaceURI = DOMNamespaces.svg;
namespaceURI = Namespaces.svg;
} else if (this._tag === 'math') {
namespaceURI = DOMNamespaces.mathml;
namespaceURI = Namespaces.mathml;
}
}
this._namespaceURI = namespaceURI;
Expand Down Expand Up @@ -557,7 +557,7 @@ ReactDOMComponent.Mixin = {
if (transaction.useCreateElement) {
var ownerDocument = hostContainerInfo._ownerDocument;
var el;
if (namespaceURI === DOMNamespaces.html) {
if (namespaceURI === Namespaces.html) {
if (this._tag === 'script') {
// Create the script via .innerHTML so its "parser-inserted" flag is
// set to true and it does not execute
Expand Down Expand Up @@ -587,7 +587,7 @@ ReactDOMComponent.Mixin = {
);
didWarnShadyDOM = true;
}
if (this._namespaceURI === DOMNamespaces.html) {
if (this._namespaceURI === Namespaces.html) {
if (
!isCustomComponentTag &&
Object.prototype.toString.call(el) ===
Expand Down
39 changes: 29 additions & 10 deletions src/renderers/shared/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@

'use strict';

var {
Namespaces,
getIntrinsicNamespace,
getChildNamespace,
} = require('DOMNamespaces');
var DOMMarkupOperations = require('DOMMarkupOperations');
var React = require('react');
var ReactControlledValuePropTypes = require('ReactControlledValuePropTypes');
Expand Down Expand Up @@ -443,6 +448,9 @@ class ReactDOMServerRenderer {
constructor(element, makeStaticMarkup) {
var children = React.isValidElement(element) ? [element] : toArray(element);
var topFrame = {
// Assume all trees start in the HTML namespace (not totally true, but
// this is what we did historically)
domNamespace: Namespaces.html,
children,
childIndex: 0,
context: emptyObject,
Expand Down Expand Up @@ -483,7 +491,7 @@ class ReactDOMServerRenderer {
if (__DEV__) {
setCurrentDebugStack(this.stack);
}
out += this.render(child, frame.context);
out += this.render(child, frame.context, frame.domNamespace);
if (__DEV__) {
// TODO: Handle reentrant server render calls. This doesn't.
resetCurrentDebugStack();
Expand All @@ -492,7 +500,7 @@ class ReactDOMServerRenderer {
return out;
}

render(child, context) {
render(child, context, parentNamespace) {
if (typeof child === 'string' || typeof child === 'number') {
var text = '' + child;
if (text === '') {
Expand All @@ -512,10 +520,11 @@ class ReactDOMServerRenderer {
return '';
} else {
if (React.isValidElement(child)) {
return this.renderDOM(child, context);
return this.renderDOM(child, context, parentNamespace);
} else {
var children = toArray(child);
var frame = {
domNamespace: parentNamespace,
children,
childIndex: 0,
context: context,
Expand All @@ -531,16 +540,25 @@ class ReactDOMServerRenderer {
}
}

renderDOM(element, context) {
renderDOM(element, context, parentNamespace) {
var tag = element.type.toLowerCase();

let namespace = parentNamespace;
if (parentNamespace === Namespaces.html) {
namespace = getIntrinsicNamespace(tag);
}

if (__DEV__) {
warning(
tag === element.type,
'<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
'in React.',
element.type,
);
if (namespace === Namespaces.html) {
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
warning(
tag === element.type,
'<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
'in React.',
element.type,
);
}
}

validateDangerousTag(tag);
Expand Down Expand Up @@ -801,6 +819,7 @@ class ReactDOMServerRenderer {
children = toArray(props.children);
}
var frame = {
domNamespace: getChildNamespace(parentNamespace, element.type),
tag,
children,
childIndex: 0,
Expand Down