diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 611dcac391..809da38734 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -683,15 +683,10 @@ src/renderers/dom/shared/__tests__/CSSProperty-test.js * should generate browser prefixes for its `isUnitlessNumber` src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js -* should create markup for simple styles -* should ignore undefined styles -* should ignore null styles -* should return null for no styles * should automatically append `px` to relevant styles * should trim values * should not append `px` to styles that might need a number * should create vendor-prefixed markup correctly -* should create markup with unitless css custom property * should set style attribute when styles exist * should not set style attribute when no styles exist * should warn when using hyphenated style names @@ -704,13 +699,6 @@ src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should not add units to CSS custom properties src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js -* should create markup for simple properties -* should work with the id attribute -* should create markup for boolean properties -* should create markup for booleanish properties -* should create markup for custom attributes -* should create markup for numeric properties -* should allow custom properties on web components * should set values as properties by default * should set values as attributes if necessary * should set values as namespace attributes if necessary @@ -728,7 +716,6 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should remove attributes for normal properties * should not remove attributes for special properties * should not leave all options selected when deleting multiple -* should support custom attributes src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js * should store a listener correctly @@ -895,11 +882,6 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders a blank div with clean client render * renders a blank div with client render on top of good server markup * renders a blank div with client render on top of bad server markup -* renders a div with inline styles with server string render -* renders a div with inline styles with server stream render -* renders a div with inline styles with clean client render -* renders a div with inline styles with client render on top of good server markup -* renders a div with inline styles with client render on top of bad server markup * renders a self-closing tag with server string render * renders a self-closing tag with server stream render * renders a self-closing tag with clean client render @@ -930,6 +912,11 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders string prop with false value with clean client render * renders string prop with false value with client render on top of good server markup * renders string prop with false value with client render on top of bad server markup +* renders no string prop with null value with server string render +* renders no string prop with null value with server stream render +* renders no string prop with null value with clean client render +* renders no string prop with null value with client render on top of good server markup +* renders no string prop with null value with client render on top of bad server markup * renders boolean prop with true value with server string render * renders boolean prop with true value with server stream render * renders boolean prop with true value with clean client render @@ -975,6 +962,11 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders boolean prop with zero value with clean client render * renders boolean prop with zero value with client render on top of good server markup * renders boolean prop with zero value with client render on top of bad server markup +* renders no boolean prop with null value with server string render +* renders no boolean prop with null value with server stream render +* renders no boolean prop with null value with clean client render +* renders no boolean prop with null value with client render on top of good server markup +* renders no boolean prop with null value with client render on top of bad server markup * renders download prop with true value with server string render * renders download prop with true value with server stream render * renders download prop with true value with clean client render @@ -990,11 +982,31 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders download prop with string value with clean client render * renders download prop with string value with client render on top of good server markup * renders download prop with string value with client render on top of bad server markup +* renders download prop with string "false" value with server string render +* renders download prop with string "false" value with server stream render +* renders download prop with string "false" value with clean client render +* renders download prop with string "false" value with client render on top of good server markup +* renders download prop with string "false" value with client render on top of bad server markup * renders download prop with string "true" value with server string render * renders download prop with string "true" value with server stream render * renders download prop with string "true" value with clean client render * renders download prop with string "true" value with client render on top of good server markup * renders download prop with string "true" value with client render on top of bad server markup +* renders download prop with number 0 value with server string render +* renders download prop with number 0 value with server stream render +* renders download prop with number 0 value with clean client render +* renders download prop with number 0 value with client render on top of good server markup +* renders download prop with number 0 value with client render on top of bad server markup +* renders no download prop with null value with server string render +* renders no download prop with null value with server stream render +* renders no download prop with null value with clean client render +* renders no download prop with null value with client render on top of good server markup +* renders no download prop with null value with client render on top of bad server markup +* renders no download prop with undefined value with server string render +* renders no download prop with undefined value with server stream render +* renders no download prop with undefined value with clean client render +* renders no download prop with undefined value with client render on top of good server markup +* renders no download prop with undefined value with client render on top of bad server markup * renders className prop with string value with server string render * renders className prop with string value with server stream render * renders className prop with string value with clean client render @@ -1015,6 +1027,11 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders className prop with false value with clean client render * renders className prop with false value with client render on top of good server markup * renders className prop with false value with client render on top of bad server markup +* renders no className prop with null value with server string render +* renders no className prop with null value with server stream render +* renders no className prop with null value with clean client render +* renders no className prop with null value with client render on top of good server markup +* renders no className prop with null value with client render on top of bad server markup * renders htmlFor with string value with server string render * renders htmlFor with string value with server stream render * renders htmlFor with string value with clean client render @@ -1025,16 +1042,36 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders htmlFor with an empty string with clean client render * renders htmlFor with an empty string with client render on top of good server markup * renders htmlFor with an empty string with client render on top of bad server markup -* renders className prop with true value with server string render -* renders className prop with true value with server stream render -* renders className prop with true value with clean client render -* renders className prop with true value with client render on top of good server markup -* renders className prop with true value with client render on top of bad server markup -* renders className prop with false value with server string render -* renders className prop with false value with server stream render -* renders className prop with false value with clean client render -* renders className prop with false value with client render on top of good server markup -* renders className prop with false value with client render on top of bad server markup +* renders htmlFor prop with true value with server string render +* renders htmlFor prop with true value with server stream render +* renders htmlFor prop with true value with clean client render +* renders htmlFor prop with true value with client render on top of good server markup +* renders htmlFor prop with true value with client render on top of bad server markup +* renders htmlFor prop with false value with server string render +* renders htmlFor prop with false value with server stream render +* renders htmlFor prop with false value with clean client render +* renders htmlFor prop with false value with client render on top of good server markup +* renders htmlFor prop with false value with client render on top of bad server markup +* renders no htmlFor prop with null value with server string render +* renders no htmlFor prop with null value with server stream render +* renders no htmlFor prop with null value with clean client render +* renders no htmlFor prop with null value with client render on top of good server markup +* renders no htmlFor prop with null value with client render on top of bad server markup +* renders positive numeric property with positive value with server string render +* renders positive numeric property with positive value with server stream render +* renders positive numeric property with positive value with clean client render +* renders positive numeric property with positive value with client render on top of good server markup +* renders positive numeric property with positive value with client render on top of bad server markup +* renders no positive numeric property with zero value with server string render +* renders no positive numeric property with zero value with server stream render +* renders no positive numeric property with zero value with clean client render +* renders no positive numeric property with zero value with client render on top of good server markup +* renders no positive numeric property with zero value with client render on top of bad server markup +* renders numeric property with zero value with server string render +* renders numeric property with zero value with server stream render +* renders numeric property with zero value with clean client render +* renders numeric property with zero value with client render on top of good server markup +* renders numeric property with zero value with client render on top of bad server markup * renders no ref attribute with server string render * renders no ref attribute with server stream render * renders no ref attribute with clean client render @@ -1055,6 +1092,51 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders no dangerouslySetInnerHTML attribute with clean client render * renders no dangerouslySetInnerHTML attribute with client render on top of good server markup * renders no dangerouslySetInnerHTML attribute with client render on top of bad server markup +* renders simple styles with server string render +* renders simple styles with server stream render +* renders simple styles with clean client render +* renders simple styles with client render on top of good server markup +* renders simple styles with client render on top of bad server markup +* renders relevant styles with px with server string render +* renders relevant styles with px with server stream render +* renders relevant styles with px with clean client render +* renders relevant styles with px with client render on top of good server markup +* renders relevant styles with px with client render on top of bad server markup +* renders custom properties with server string render +* renders custom properties with server stream render +* renders custom properties with clean client render +* renders custom properties with client render on top of good server markup +* renders custom properties with client render on top of bad server markup +* renders no undefined styles with server string render +* renders no undefined styles with server stream render +* renders no undefined styles with clean client render +* renders no undefined styles with client render on top of good server markup +* renders no undefined styles with client render on top of bad server markup +* renders no null styles with server string render +* renders no null styles with server stream render +* renders no null styles with clean client render +* renders no null styles with client render on top of good server markup +* renders no null styles with client render on top of bad server markup +* renders no empty styles with server string render +* renders no empty styles with server stream render +* renders no empty styles with clean client render +* renders no empty styles with client render on top of good server markup +* renders no empty styles with client render on top of bad server markup +* renders simple strings with server string render +* renders simple strings with server stream render +* renders simple strings with clean client render +* renders simple strings with client render on top of good server markup +* renders simple strings with client render on top of bad server markup +* renders aria string prop with false value with server string render +* renders aria string prop with false value with server stream render +* renders aria string prop with false value with clean client render +* renders aria string prop with false value with client render on top of good server markup +* renders aria string prop with false value with client render on top of bad server markup +* renders no aria prop with null value with server string render +* renders no aria prop with null value with server stream render +* renders no aria prop with null value with clean client render +* renders no aria prop with null value with client render on top of good server markup +* renders no aria prop with null value with client render on top of bad server markup * renders no unknown attributes with server string render * renders no unknown attributes with server stream render * renders no unknown attributes with clean client render @@ -1065,6 +1147,11 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders unknown data- attributes with clean client render * renders unknown data- attributes with client render on top of good server markup * renders unknown data- attributes with client render on top of bad server markup +* renders no unknown data- attributes with null value with server string render +* renders no unknown data- attributes with null value with server stream render +* renders no unknown data- attributes with null value with clean client render +* renders no unknown data- attributes with null value with client render on top of good server markup +* renders no unknown data- attributes with null value with client render on top of bad server markup * renders no unknown attributes for non-standard elements with server string render * renders no unknown attributes for non-standard elements with server stream render * renders no unknown attributes for non-standard elements with clean client render @@ -1075,11 +1162,21 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders unknown attributes for custom elements with clean client render * renders unknown attributes for custom elements with client render on top of good server markup * renders unknown attributes for custom elements with client render on top of bad server markup +* renders no unknown attributes for custom elements with null value with server string render +* renders no unknown attributes for custom elements with null value with server stream render +* renders no unknown attributes for custom elements with null value with clean client render +* renders no unknown attributes for custom elements with null value with client render on top of good server markup +* renders no unknown attributes for custom elements with null value with client render on top of bad server markup * renders unknown attributes for custom elements using is with server string render * renders unknown attributes for custom elements using is with server stream render * renders unknown attributes for custom elements using is with clean client render * renders unknown attributes for custom elements using is with client render on top of good server markup * renders unknown attributes for custom elements using is with client render on top of bad server markup +* renders no unknown attributes for custom elements using is with null value with server string render +* renders no unknown attributes for custom elements using is with null value with server stream render +* renders no unknown attributes for custom elements using is with null value with clean client render +* renders no unknown attributes for custom elements using is with null value with client render on top of good server markup +* renders no unknown attributes for custom elements using is with null value with client render on top of bad server markup * renders no HTML events with server string render * renders no HTML events with server stream render * renders no HTML events with clean client render @@ -1563,6 +1660,11 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * can distinguish an empty component from a dom node * can distinguish an empty component from an empty text component * should error reconnecting a div with different dangerouslySetInnerHTML +* renders injected attributes with server string render +* renders injected attributes with server stream render +* renders injected attributes with clean client render +* renders injected attributes with client render on top of good server markup +* renders injected attributes with client render on top of bad server markup src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * updates a mounted text component in place diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index 185922450e..34f5166af3 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -65,20 +65,20 @@ "gzip": 81957 }, "react-dom-server.development.js (UMD_DEV)": { - "size": 152929, - "gzip": 37451 + "size": 119540, + "gzip": 30394 }, "react-dom-server.production.min.js (UMD_PROD)": { - "size": 25022, - "gzip": 9357 + "size": 23004, + "gzip": 8753 }, "react-dom-server.development.js (NODE_DEV)": { - "size": 120074, - "gzip": 29764 + "size": 88637, + "gzip": 22952 }, "react-dom-server.production.min.js (NODE_PROD)": { - "size": 22559, - "gzip": 8401 + "size": 20744, + "gzip": 7879 }, "ReactDOMServerStream-dev.js (FB_DEV)": { "size": 264750, @@ -181,20 +181,20 @@ "gzip": 50920 }, "ReactDOMServer-dev.js (FB_DEV)": { - "size": 119528, - "gzip": 29719 + "size": 88149, + "gzip": 22908 }, "ReactDOMServer-prod.js (FB_PROD)": { - "size": 60237, - "gzip": 16296 + "size": 53879, + "gzip": 14872 }, "react-dom-node-stream.development.js (NODE_DEV)": { - "size": 121768, - "gzip": 30291 + "size": 90331, + "gzip": 23443 }, "react-dom-node-stream.production.min.js (NODE_PROD)": { - "size": 23522, - "gzip": 8727 + "size": 21682, + "gzip": 8219 }, "ReactDOMNodeStream-dev.js (FB_DEV)": { "size": 264918, diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index 8e285f5eea..25f95ef353 100644 --- a/src/renderers/dom/shared/CSSPropertyOperations.js +++ b/src/renderers/dom/shared/CSSPropertyOperations.js @@ -14,21 +14,13 @@ var CSSProperty = require('CSSProperty'); var ExecutionEnvironment = require('fbjs/lib/ExecutionEnvironment'); -var camelizeStyleName = require('fbjs/lib/camelizeStyleName'); var dangerousStyleValue = require('dangerousStyleValue'); -var getComponentName = require('getComponentName'); var hyphenateStyleName = require('fbjs/lib/hyphenateStyleName'); -var memoizeStringOnly = require('fbjs/lib/memoizeStringOnly'); -var warning = require('fbjs/lib/warning'); if (__DEV__) { - var {getCurrentFiberOwnerName} = require('ReactDebugCurrentFiber'); + var warnValidStyle = require('warnValidStyle'); } -var processStyleName = memoizeStringOnly(function(styleName) { - return hyphenateStyleName(styleName); -}); - var hasShorthandPropertyBug = false; if (ExecutionEnvironment.canUseDOM) { var tempStyle = document.createElement('div').style; @@ -40,190 +32,15 @@ if (ExecutionEnvironment.canUseDOM) { } } -if (__DEV__) { - // 'msTransform' is correct, but the other prefixes should be capitalized - var badVendoredStyleNamePattern = /^(?:webkit|moz|o)[A-Z]/; - - // style values shouldn't contain a semicolon - var badStyleValueWithSemicolonPattern = /;\s*$/; - - var warnedStyleNames = {}; - var warnedStyleValues = {}; - var warnedForNaNValue = false; - var warnedForInfinityValue = false; - - var warnHyphenatedStyleName = function(name, owner) { - if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) { - return; - } - - warnedStyleNames[name] = true; - warning( - false, - 'Unsupported style property %s. Did you mean %s?%s', - name, - camelizeStyleName(name), - checkRenderMessage(owner), - ); - }; - - var warnBadVendoredStyleName = function(name, owner) { - if (warnedStyleNames.hasOwnProperty(name) && warnedStyleNames[name]) { - return; - } - - warnedStyleNames[name] = true; - warning( - false, - 'Unsupported vendor-prefixed style property %s. Did you mean %s?%s', - name, - name.charAt(0).toUpperCase() + name.slice(1), - checkRenderMessage(owner), - ); - }; - - var warnStyleValueWithSemicolon = function(name, value, owner) { - if (warnedStyleValues.hasOwnProperty(value) && warnedStyleValues[value]) { - return; - } - - warnedStyleValues[value] = true; - warning( - false, - "Style property values shouldn't contain a semicolon.%s " + - 'Try "%s: %s" instead.', - checkRenderMessage(owner), - name, - value.replace(badStyleValueWithSemicolonPattern, ''), - ); - }; - - var warnStyleValueIsNaN = function(name, value, owner) { - if (warnedForNaNValue) { - return; - } - - warnedForNaNValue = true; - warning( - false, - '`NaN` is an invalid value for the `%s` css style property.%s', - name, - checkRenderMessage(owner), - ); - }; - - var warnStyleValueIsInfinity = function(name, value, owner) { - if (warnedForInfinityValue) { - return; - } - - warnedForInfinityValue = true; - warning( - false, - '`Infinity` is an invalid value for the `%s` css style property.%s', - name, - checkRenderMessage(owner), - ); - }; - - var checkRenderMessage = function(owner) { - var ownerName; - if (owner != null) { - // Stack passes the owner manually all the way to CSSPropertyOperations. - ownerName = getComponentName(owner); - } else { - // Fiber doesn't pass it but uses ReactDebugCurrentFiber to track it. - // It is only enabled in development and tracks host components too. - ownerName = getCurrentFiberOwnerName(); - // TODO: also report the stack. - } - if (ownerName) { - return '\n\nCheck the render method of `' + ownerName + '`.'; - } - return ''; - }; - - /** - * @param {string} name - * @param {*} value - * @param {ReactDOMComponent} component - */ - var warnValidStyle = function(name, value, component) { - var owner; - if (component) { - owner = component._currentElement._owner; - } - if (name.indexOf('-') > -1) { - warnHyphenatedStyleName(name, owner); - } else if (badVendoredStyleNamePattern.test(name)) { - warnBadVendoredStyleName(name, owner); - } else if (badStyleValueWithSemicolonPattern.test(value)) { - warnStyleValueWithSemicolon(name, value, owner); - } - - if (typeof value === 'number') { - if (isNaN(value)) { - warnStyleValueIsNaN(name, value, owner); - } else if (!isFinite(value)) { - warnStyleValueIsInfinity(name, value, owner); - } - } - }; -} - /** * Operations for dealing with CSS properties. */ var CSSPropertyOperations = { - /** - * Serializes a mapping of style properties for use as inline styles: - * - * > createMarkupForStyles({width: '200px', height: 0}) - * "width:200px;height:0;" - * - * Undefined values are ignored so that declarative programming is easier. - * The result should be HTML-escaped before insertion into the DOM. - * - * @param {object} styles - * @param {ReactDOMComponent} component - * @return {?string} - */ - createMarkupForStyles: function(styles, component) { - var serialized = ''; - var delimiter = ''; - for (var styleName in styles) { - if (!styles.hasOwnProperty(styleName)) { - continue; - } - var isCustomProperty = styleName.indexOf('--') === 0; - var styleValue = styles[styleName]; - if (__DEV__) { - if (!isCustomProperty) { - warnValidStyle(styleName, styleValue, component); - } - } - if (styleValue != null) { - serialized += delimiter + processStyleName(styleName) + ':'; - serialized += dangerousStyleValue( - styleName, - styleValue, - isCustomProperty, - ); - - delimiter = ';'; - } - } - return serialized || null; - }, - /** * This creates a string that is expected to be equivalent to the style * attribute generated by server-side rendering. It by-passes warnings and * security checks so it's not safe to use this value for anything other than - * comparison. It is only used in DEV for SSR validation. This is duplicated - * from createMarkupForStyles because createMarkupForStyles is expected to - * move out of the client-side renderer and it would be nice to make a clean - * break. + * comparison. It is only used in DEV for SSR validation. */ createDangerousStringForStyles: function(styles) { if (__DEV__) { diff --git a/src/renderers/dom/shared/DOMMarkupOperations.js b/src/renderers/dom/shared/DOMMarkupOperations.js new file mode 100644 index 0000000000..1f34e0d0fe --- /dev/null +++ b/src/renderers/dom/shared/DOMMarkupOperations.js @@ -0,0 +1,125 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule DOMMarkupOperations + */ + +'use strict'; + +var DOMProperty = require('DOMProperty'); + +var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); +var warning = require('fbjs/lib/warning'); + +// isAttributeNameSafe() is currently duplicated in DOMPropertyOperations. +// TODO: Find a better place for this. +var VALID_ATTRIBUTE_NAME_REGEX = new RegExp( + '^[' + + DOMProperty.ATTRIBUTE_NAME_START_CHAR + + '][' + + DOMProperty.ATTRIBUTE_NAME_CHAR + + ']*$', +); +var illegalAttributeNameCache = {}; +var validatedAttributeNameCache = {}; +function isAttributeNameSafe(attributeName) { + if (validatedAttributeNameCache.hasOwnProperty(attributeName)) { + return true; + } + if (illegalAttributeNameCache.hasOwnProperty(attributeName)) { + return false; + } + if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) { + validatedAttributeNameCache[attributeName] = true; + return true; + } + illegalAttributeNameCache[attributeName] = true; + warning(false, 'Invalid attribute name: `%s`', attributeName); + return false; +} + +// shouldIgnoreValue() is currently duplicated in DOMPropertyOperations. +// TODO: Find a better place for this. +function shouldIgnoreValue(propertyInfo, value) { + return ( + value == null || + (propertyInfo.hasBooleanValue && !value) || + (propertyInfo.hasNumericValue && isNaN(value)) || + (propertyInfo.hasPositiveNumericValue && value < 1) || + (propertyInfo.hasOverloadedBooleanValue && value === false) + ); +} + +/** + * Operations for dealing with DOM properties. + */ +var DOMMarkupOperations = { + /** + * Creates markup for the ID property. + * + * @param {string} id Unescaped ID. + * @return {string} Markup string. + */ + createMarkupForID: function(id) { + return ( + DOMProperty.ID_ATTRIBUTE_NAME + '=' + quoteAttributeValueForBrowser(id) + ); + }, + + createMarkupForRoot: function() { + return DOMProperty.ROOT_ATTRIBUTE_NAME + '=""'; + }, + + /** + * Creates markup for a property. + * + * @param {string} name + * @param {*} value + * @return {?string} Markup string, or null if the property was invalid. + */ + createMarkupForProperty: function(name, value) { + var propertyInfo = DOMProperty.properties.hasOwnProperty(name) + ? DOMProperty.properties[name] + : null; + if (propertyInfo) { + if (shouldIgnoreValue(propertyInfo, value)) { + return ''; + } + var attributeName = propertyInfo.attributeName; + if ( + propertyInfo.hasBooleanValue || + (propertyInfo.hasOverloadedBooleanValue && value === true) + ) { + return attributeName + '=""'; + } + return attributeName + '=' + quoteAttributeValueForBrowser(value); + } else if (DOMProperty.isCustomAttribute(name)) { + if (value == null) { + return ''; + } + return name + '=' + quoteAttributeValueForBrowser(value); + } + return null; + }, + + /** + * Creates markup for a custom property. + * + * @param {string} name + * @param {*} value + * @return {string} Markup string, or empty string if the property was invalid. + */ + createMarkupForCustomAttribute: function(name, value) { + if (!isAttributeNameSafe(name) || value == null) { + return ''; + } + return name + '=' + quoteAttributeValueForBrowser(value); + }, +}; + +module.exports = DOMMarkupOperations; diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 94b751e67d..5a4a7613aa 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -15,9 +15,10 @@ var DOMProperty = require('DOMProperty'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactInstrumentation = require('ReactInstrumentation'); -var quoteAttributeValueForBrowser = require('quoteAttributeValueForBrowser'); var warning = require('fbjs/lib/warning'); +// isAttributeNameSafe() is currently duplicated in DOMMarkupOperations. +// TODO: Find a better place for this. var VALID_ATTRIBUTE_NAME_REGEX = new RegExp( '^[' + DOMProperty.ATTRIBUTE_NAME_START_CHAR + @@ -27,7 +28,6 @@ var VALID_ATTRIBUTE_NAME_REGEX = new RegExp( ); var illegalAttributeNameCache = {}; var validatedAttributeNameCache = {}; - function isAttributeNameSafe(attributeName) { if (validatedAttributeNameCache.hasOwnProperty(attributeName)) { return true; @@ -44,6 +44,8 @@ function isAttributeNameSafe(attributeName) { return false; } +// shouldIgnoreValue() is currently duplicated in DOMMarkupOperations. +// TODO: Find a better place for this. function shouldIgnoreValue(propertyInfo, value) { return ( value == null || @@ -58,76 +60,14 @@ function shouldIgnoreValue(propertyInfo, value) { * Operations for dealing with DOM properties. */ var DOMPropertyOperations = { - /** - * Creates markup for the ID property. - * - * @param {string} id Unescaped ID. - * @return {string} Markup string. - */ - createMarkupForID: function(id) { - return ( - DOMProperty.ID_ATTRIBUTE_NAME + '=' + quoteAttributeValueForBrowser(id) - ); - }, - setAttributeForID: function(node, id) { node.setAttribute(DOMProperty.ID_ATTRIBUTE_NAME, id); }, - createMarkupForRoot: function() { - return DOMProperty.ROOT_ATTRIBUTE_NAME + '=""'; - }, - setAttributeForRoot: function(node) { node.setAttribute(DOMProperty.ROOT_ATTRIBUTE_NAME, ''); }, - /** - * Creates markup for a property. - * - * @param {string} name - * @param {*} value - * @return {?string} Markup string, or null if the property was invalid. - */ - createMarkupForProperty: function(name, value) { - var propertyInfo = DOMProperty.properties.hasOwnProperty(name) - ? DOMProperty.properties[name] - : null; - if (propertyInfo) { - if (shouldIgnoreValue(propertyInfo, value)) { - return ''; - } - var attributeName = propertyInfo.attributeName; - if ( - propertyInfo.hasBooleanValue || - (propertyInfo.hasOverloadedBooleanValue && value === true) - ) { - return attributeName + '=""'; - } - return attributeName + '=' + quoteAttributeValueForBrowser(value); - } else if (DOMProperty.isCustomAttribute(name)) { - if (value == null) { - return ''; - } - return name + '=' + quoteAttributeValueForBrowser(value); - } - return null; - }, - - /** - * Creates markup for a custom property. - * - * @param {string} name - * @param {*} value - * @return {string} Markup string, or empty string if the property was invalid. - */ - createMarkupForCustomAttribute: function(name, value) { - if (!isAttributeNameSafe(name) || value == null) { - return ''; - } - return name + '=' + quoteAttributeValueForBrowser(value); - }, - /** * Get the value for a property on a node. Only used in DEV for SSR validation. * The "expected" argument is used as a hint of what the expected value is. @@ -153,6 +93,12 @@ var DOMPropertyOperations = { if (value === '') { return true; } + if (shouldIgnoreValue(propertyInfo, expected)) { + return value; + } + if (value === '' + expected) { + return expected; + } return value; } } else if (node.hasAttribute(attributeName)) { diff --git a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js index 065ac059dc..2e3f9fc603 100644 --- a/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js @@ -16,97 +16,47 @@ var ReactDOM = require('react-dom'); var ReactDOMServer = require('react-dom/server'); describe('CSSPropertyOperations', () => { - var CSSPropertyOperations; - - beforeEach(() => { - jest.resetModules(); - CSSPropertyOperations = require('CSSPropertyOperations'); - }); - - it('should create markup for simple styles', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - backgroundColor: '#3b5998', - display: 'none', - }), - ).toBe('background-color:#3b5998;display:none'); - }); - - it('should ignore undefined styles', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - backgroundColor: undefined, - display: 'none', - }), - ).toBe('display:none'); - }); - - it('should ignore null styles', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - backgroundColor: null, - display: 'none', - }), - ).toBe('display:none'); - }); - - it('should return null for no styles', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - backgroundColor: null, - display: null, - }), - ).toBe(null); - }); - it('should automatically append `px` to relevant styles', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - left: 0, - margin: 16, - opacity: 0.5, - padding: '4px', - }), - ).toBe('left:0;margin:16px;opacity:0.5;padding:4px'); + var styles = { + left: 0, + margin: 16, + opacity: 0.5, + padding: '4px', + }; + var div =
; + var html = ReactDOMServer.renderToString(div); + expect(html).toContain('"left:0;margin:16px;opacity:0.5;padding:4px"'); }); it('should trim values', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - left: '16 ', - opacity: 0.5, - right: ' 4 ', - }), - ).toBe('left:16;opacity:0.5;right:4'); + var styles = { + left: '16 ', + opacity: 0.5, + right: ' 4 ', + }; + var div =
; + var html = ReactDOMServer.renderToString(div); + expect(html).toContain('"left:16;opacity:0.5;right:4"'); }); it('should not append `px` to styles that might need a number', () => { - var CSSProperty = require('CSSProperty'); - var unitlessProperties = Object.keys(CSSProperty.isUnitlessNumber); - unitlessProperties.forEach(function(property) { - var styles = {}; - styles[property] = 1; - expect(CSSPropertyOperations.createMarkupForStyles(styles)).toMatch( - /:1$/, - ); - }); + var styles = { + flex: 0, + opacity: 0.5, + }; + var div =
; + var html = ReactDOMServer.renderToString(div); + expect(html).toContain('"flex:0;opacity:0.5"'); }); it('should create vendor-prefixed markup correctly', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - msTransition: 'none', - MozTransition: 'none', - }), - ).toBe('-ms-transition:none;-moz-transition:none'); - }); - - it('should create markup with unitless css custom property', () => { - expect( - CSSPropertyOperations.createMarkupForStyles({ - '--foo': 5, - }), - ).toBe('--foo:5'); + var styles = { + msTransition: 'none', + MozTransition: 'none', + }; + var div =
; + var html = ReactDOMServer.renderToString(div); + expect(html).toContain('"-ms-transition:none;-moz-transition:none"'); }); it('should set style attribute when styles exist', () => { diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 954780de8f..eba36070cb 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -26,122 +26,6 @@ describe('DOMPropertyOperations', () => { ReactDOMComponentTree = require('ReactDOMComponentTree'); }); - describe('createMarkupForProperty', () => { - it('should create markup for simple properties', () => { - expect( - DOMPropertyOperations.createMarkupForProperty('name', 'simple'), - ).toBe('name="simple"'); - - expect(DOMPropertyOperations.createMarkupForProperty('name', false)).toBe( - 'name="false"', - ); - - expect(DOMPropertyOperations.createMarkupForProperty('name', null)).toBe( - '', - ); - }); - - it('should work with the id attribute', () => { - expect( - DOMPropertyOperations.createMarkupForProperty('id', 'simple'), - ).toBe('id="simple"'); - }); - - it('should create markup for boolean properties', () => { - expect( - DOMPropertyOperations.createMarkupForProperty('checked', 'simple'), - ).toBe('checked=""'); - - expect( - DOMPropertyOperations.createMarkupForProperty('checked', true), - ).toBe('checked=""'); - - expect( - DOMPropertyOperations.createMarkupForProperty('checked', false), - ).toBe(''); - - expect( - DOMPropertyOperations.createMarkupForProperty('scoped', true), - ).toBe('scoped=""'); - }); - - it('should create markup for booleanish properties', () => { - expect( - DOMPropertyOperations.createMarkupForProperty('download', 'simple'), - ).toBe('download="simple"'); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', true), - ).toBe('download=""'); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', 'true'), - ).toBe('download="true"'); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', false), - ).toBe(''); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', 'false'), - ).toBe('download="false"'); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', undefined), - ).toBe(''); - - expect( - DOMPropertyOperations.createMarkupForProperty('download', null), - ).toBe(''); - - expect(DOMPropertyOperations.createMarkupForProperty('download', 0)).toBe( - 'download="0"', - ); - }); - - it('should create markup for custom attributes', () => { - expect( - DOMPropertyOperations.createMarkupForProperty('aria-label', 'simple'), - ).toBe('aria-label="simple"'); - - expect( - DOMPropertyOperations.createMarkupForProperty('aria-label', false), - ).toBe('aria-label="false"'); - - expect( - DOMPropertyOperations.createMarkupForProperty('aria-label', null), - ).toBe(''); - }); - - it('should create markup for numeric properties', () => { - expect(DOMPropertyOperations.createMarkupForProperty('start', 5)).toBe( - 'start="5"', - ); - - expect(DOMPropertyOperations.createMarkupForProperty('start', 0)).toBe( - 'start="0"', - ); - - expect(DOMPropertyOperations.createMarkupForProperty('size', 0)).toBe(''); - - expect(DOMPropertyOperations.createMarkupForProperty('size', 1)).toBe( - 'size="1"', - ); - }); - }); - - describe('createMarkupForProperty', () => { - it('should allow custom properties on web components', () => { - expect( - DOMPropertyOperations.createMarkupForCustomAttribute('awesomeness', 5), - ).toBe('awesomeness="5"'); - - expect( - DOMPropertyOperations.createMarkupForCustomAttribute('dev', 'jim'), - ).toBe('dev="jim"'); - }); - }); - describe('setValueForProperty', () => { var stubNode; var stubInstance; @@ -377,51 +261,4 @@ describe('DOMPropertyOperations', () => { ); }); }); - - describe('injectDOMPropertyConfig', () => { - it('should support custom attributes', () => { - // foobar does not exist yet - expect( - DOMPropertyOperations.createMarkupForProperty('foobar', 'simple'), - ).toBe(null); - - // foo-* does not exist yet - expect( - DOMPropertyOperations.createMarkupForProperty('foo-xyz', 'simple'), - ).toBe(null); - - // inject foobar DOM property - DOMProperty.injection.injectDOMPropertyConfig({ - isCustomAttribute: function(name) { - return name.indexOf('foo-') === 0; - }, - Properties: {foobar: null}, - }); - - // Ensure old attributes still work - expect( - DOMPropertyOperations.createMarkupForProperty('name', 'simple'), - ).toBe('name="simple"'); - expect( - DOMPropertyOperations.createMarkupForProperty('data-name', 'simple'), - ).toBe('data-name="simple"'); - - // foobar should work - expect( - DOMPropertyOperations.createMarkupForProperty('foobar', 'simple'), - ).toBe('foobar="simple"'); - - // foo-* should work - expect( - DOMPropertyOperations.createMarkupForProperty('foo-xyz', 'simple'), - ).toBe('foo-xyz="simple"'); - - // It should complain about double injections. - expect(function() { - DOMProperty.injection.injectDOMPropertyConfig({ - Properties: {foobar: null}, - }); - }).toThrow(); - }); - }); }); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 08fc0ebe95..220c71f9ab 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -285,6 +285,7 @@ function expectMarkupMismatch(serverElement, clientElement) { // error, you want to see the error show up both on server and client. Unfortunately, // React refuses to issue the same error twice to avoid clogging up the console. // To get around this, we must reload React modules in between server and client render. +let onAfterResetModules = null; function resetModules() { jest.resetModuleRegistry(); PropTypes = require('prop-types'); @@ -296,6 +297,12 @@ function resetModules() { ReactTestUtils = require('react-dom/test-utils'); // TODO: can we express this test with only public API? ExecutionEnvironment = require('ExecutionEnvironment'); + + // TODO: this is a hack for testing dynamic injection. Remove this when we decide + // how to do static injection instead. + if (typeof onAfterResetModules === 'function') { + onAfterResetModules(); + } } describe('ReactDOMServerIntegration', () => { @@ -311,12 +318,6 @@ describe('ReactDOMServerIntegration', () => { expect(e.tagName).toBe('DIV'); }); - itRenders('a div with inline styles', async render => { - const e = await render(
); - expect(e.style.color).toBe('red'); - expect(e.style.width).toBe('30px'); - }); - itRenders('a self-closing tag', async render => { const e = await render(
); expect(e.tagName).toBe('BR'); @@ -352,6 +353,11 @@ describe('ReactDOMServerIntegration', () => { const e = await render(); expect(e.getAttribute('href')).toBe('false'); }); + + itRenders('no string prop with null value', async render => { + const e = await render(
); + expect(e.hasAttribute('width')).toBe(false); + }); }); describe('boolean properties', function() { @@ -407,6 +413,11 @@ describe('ReactDOMServerIntegration', () => { const e = await render(