From e10694ae9dc02b8b0679b40a87aecb38e5ddc907 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Thu, 24 Aug 2017 07:26:43 -0400 Subject: [PATCH 01/36] Remove HTMLPropertyConfig entries for non-boolean values When we originally removed attributes from the whitelist, we assumed a few attributes were string booleans, but they are not: Autocomplete ("on", "off") https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocomplete Autocapitalize ("none", "sentence", "words", ...) https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocapitalize Autocorrect ("on", "off") https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autocorrect Autosave (string) https://developer.apple.com/library/content/documentation/AppleApplications/Reference/SafariHTMLRef/Articles/Attributes.html#autosave --- src/renderers/dom/shared/HTMLDOMPropertyConfig.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index ed3808c7f7dc7..2a8017cfc64fa 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -78,19 +78,12 @@ var HTMLDOMPropertyConfig = { value: 0, // The following attributes expect boolean values. They must be in // the whitelist to allow boolean attribute assignment: - autoComplete: 0, // IE only true/false iFrame attribute // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx allowTransparency: 0, contentEditable: 0, draggable: 0, spellCheck: 0, - // autoCapitalize and autoCorrect are supported in Mobile Safari for - // keyboard hints. - autoCapitalize: 0, - autoCorrect: 0, - // autoSave allows WebKit/Blink to persist values of input fields on page reloads - autoSave: 0, }, DOMAttributeNames: { acceptCharset: 'accept-charset', From 711bafe24be4805594c13bd36311e12d5d33209f Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 28 Aug 2017 21:53:09 -0400 Subject: [PATCH 02/36] Only HAS_BOOLEAN_VALUE attribute flag can assign booleans --- .../dom/shared/HTMLDOMPropertyConfig.js | 14 ++++++-------- .../dom/shared/SVGDOMPropertyConfig.js | 17 +++++++++++------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 2a8017cfc64fa..a4c9248ff5fd2 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -27,6 +27,9 @@ var HTMLDOMPropertyConfig = { // name warnings. Properties: { allowFullScreen: HAS_BOOLEAN_VALUE, + // IE only true/false iFrame attribute + // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx + allowTransparency: HAS_BOOLEAN_VALUE, // specifies target context for links with `preload` type async: HAS_BOOLEAN_VALUE, // autoFocus is polyfilled/normalized by AutoFocusUtils @@ -35,11 +38,13 @@ var HTMLDOMPropertyConfig = { capture: HAS_BOOLEAN_VALUE, checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, cols: HAS_POSITIVE_NUMERIC_VALUE, + contentEditable: HAS_BOOLEAN_VALUE, controls: HAS_BOOLEAN_VALUE, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, + draggable: HAS_BOOLEAN_VALUE, formNoValidate: HAS_BOOLEAN_VALUE, hidden: HAS_BOOLEAN_VALUE, loop: HAS_BOOLEAN_VALUE, @@ -62,6 +67,7 @@ var HTMLDOMPropertyConfig = { start: HAS_NUMERIC_VALUE, // support for projecting regular DOM Elements via V1 named slots ( shadow dom ) span: HAS_POSITIVE_NUMERIC_VALUE, + spellCheck: HAS_BOOLEAN_VALUE, // Style must be explicitly set in the attribute list. React components // expect a style object style: 0, @@ -76,14 +82,6 @@ var HTMLDOMPropertyConfig = { httpEquiv: 0, // Attributes with mutation methods must be specified in the whitelist value: 0, - // The following attributes expect boolean values. They must be in - // the whitelist to allow boolean attribute assignment: - // IE only true/false iFrame attribute - // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx - allowTransparency: 0, - contentEditable: 0, - draggable: 0, - spellCheck: 0, }, DOMAttributeNames: { acceptCharset: 'accept-charset', diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index 46db12aa4250d..0df75acf32ad8 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -11,6 +11,10 @@ 'use strict'; +var DOMProperty = require('DOMProperty'); + +var HAS_BOOLEAN_VALUE = DOMProperty.injection.MUST_USE_PROPERTY; + var NS = { xlink: 'http://www.w3.org/1999/xlink', xml: 'http://www.w3.org/XML/1998/namespace', @@ -113,15 +117,16 @@ var ATTRS = [ 'xmlns:xlink', 'xml:lang', 'xml:space', - // The following attributes expect boolean values. They must be in - // the whitelist to allow boolean attribute assignment: - 'autoReverse', - 'externalResourcesRequired', - 'preserveAlpha', ]; var SVGDOMPropertyConfig = { - Properties: {}, + Properties: { + // The following attributes expect boolean values. They must be in + // the whitelist to allow boolean attribute assignment: + autoReverse: HAS_BOOLEAN_VALUE, + externalResourcesRequired: HAS_BOOLEAN_VALUE, + preserveAlpha: HAS_BOOLEAN_VALUE, + }, DOMAttributeNamespaces: { xlinkActuate: NS.xlink, xlinkArcrole: NS.xlink, From 4c5dfbb7b28efe6bc97d8cc86750ad8da5968efb Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 28 Aug 2017 22:03:26 -0400 Subject: [PATCH 03/36] Use a non-boolean attribute in object assignment tests --- src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 981c9d3ed2b41..ce2725cdd593f 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2097,9 +2097,9 @@ describe('ReactDOMComponent', () => { describe('Object stringification', function() { it('allows objects on known properties', function() { var el = ReactTestUtils.renderIntoDocument( -
, +
, ); - expect(el.getAttribute('allowtransparency')).toBe('[object Object]'); + expect(el.getAttribute('acceptCharset')).toBe('[object Object]'); }); it('should pass objects as attributes if they define toString', () => { From e086ff1630b1df18cc1068634ff218cb02574e86 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 28 Aug 2017 22:36:18 -0400 Subject: [PATCH 04/36] Add HAS_STRING_BOOLEAN_VALUE attribute flag --- src/renderers/dom/shared/DOMProperty.js | 10 ++++- .../dom/shared/HTMLDOMPropertyConfig.js | 9 +++-- .../dom/shared/SVGDOMPropertyConfig.js | 10 ++--- .../__tests__/ReactDOMComponent-test.js | 39 +++++++++++++++++-- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index d6ddf9c3b3ae5..5365b4bb4a75d 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -42,6 +42,7 @@ var DOMPropertyInjection = { HAS_NUMERIC_VALUE: 0x8, HAS_POSITIVE_NUMERIC_VALUE: 0x10 | 0x8, HAS_OVERLOADED_BOOLEAN_VALUE: 0x20, + HAS_STRING_BOOLEAN_VALUE: 0x40, /** * Inject some specialized knowledge about the DOM. This takes a config object @@ -103,6 +104,10 @@ var DOMPropertyInjection = { propConfig, Injection.HAS_OVERLOADED_BOOLEAN_VALUE, ), + hasStringBooleanValue: checkMask( + propConfig, + Injection.HAS_STRING_BOOLEAN_VALUE, + ), }; invariant( propertyInfo.hasBooleanValue + @@ -213,14 +218,15 @@ var DOMProperty = { switch (typeof value) { case 'boolean': if (propertyInfo) { - return true; + return ( + propertyInfo.hasBooleanValue || propertyInfo.hasStringBooleanValue + ); } var prefix = lowerCased.slice(0, 5); return prefix === 'data-' || prefix === 'aria-'; case 'undefined': case 'number': case 'string': - return true; case 'object': return true; default: diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index a4c9248ff5fd2..be07c1804fa39 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -20,6 +20,7 @@ var HAS_POSITIVE_NUMERIC_VALUE = DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE; var HAS_OVERLOADED_BOOLEAN_VALUE = DOMProperty.injection.HAS_OVERLOADED_BOOLEAN_VALUE; +var HAS_STRING_BOOLEAN_VALUE = DOMProperty.injection.HAS_STRING_BOOLEAN_VALUE; var HTMLDOMPropertyConfig = { // When adding attributes to this list, be sure to also add them to @@ -29,7 +30,7 @@ var HTMLDOMPropertyConfig = { allowFullScreen: HAS_BOOLEAN_VALUE, // IE only true/false iFrame attribute // https://msdn.microsoft.com/en-us/library/ms533072(v=vs.85).aspx - allowTransparency: HAS_BOOLEAN_VALUE, + allowTransparency: HAS_STRING_BOOLEAN_VALUE, // specifies target context for links with `preload` type async: HAS_BOOLEAN_VALUE, // autoFocus is polyfilled/normalized by AutoFocusUtils @@ -38,13 +39,13 @@ var HTMLDOMPropertyConfig = { capture: HAS_BOOLEAN_VALUE, checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, cols: HAS_POSITIVE_NUMERIC_VALUE, - contentEditable: HAS_BOOLEAN_VALUE, + contentEditable: HAS_STRING_BOOLEAN_VALUE, controls: HAS_BOOLEAN_VALUE, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, - draggable: HAS_BOOLEAN_VALUE, + draggable: HAS_STRING_BOOLEAN_VALUE, formNoValidate: HAS_BOOLEAN_VALUE, hidden: HAS_BOOLEAN_VALUE, loop: HAS_BOOLEAN_VALUE, @@ -67,7 +68,7 @@ var HTMLDOMPropertyConfig = { start: HAS_NUMERIC_VALUE, // support for projecting regular DOM Elements via V1 named slots ( shadow dom ) span: HAS_POSITIVE_NUMERIC_VALUE, - spellCheck: HAS_BOOLEAN_VALUE, + spellCheck: HAS_STRING_BOOLEAN_VALUE, // Style must be explicitly set in the attribute list. React components // expect a style object style: 0, diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index 0df75acf32ad8..c258ba0cf280f 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -13,7 +13,7 @@ var DOMProperty = require('DOMProperty'); -var HAS_BOOLEAN_VALUE = DOMProperty.injection.MUST_USE_PROPERTY; +var {HAS_STRING_BOOLEAN_VALUE} = DOMProperty.injection; var NS = { xlink: 'http://www.w3.org/1999/xlink', @@ -121,11 +121,9 @@ var ATTRS = [ var SVGDOMPropertyConfig = { Properties: { - // The following attributes expect boolean values. They must be in - // the whitelist to allow boolean attribute assignment: - autoReverse: HAS_BOOLEAN_VALUE, - externalResourcesRequired: HAS_BOOLEAN_VALUE, - preserveAlpha: HAS_BOOLEAN_VALUE, + autoReverse: HAS_STRING_BOOLEAN_VALUE, + externalResourcesRequired: HAS_STRING_BOOLEAN_VALUE, + preserveAlpha: HAS_STRING_BOOLEAN_VALUE, }, DOMAttributeNamespaces: { xlinkActuate: NS.xlink, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index ce2725cdd593f..27bec59a6edc6 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2096,10 +2096,8 @@ describe('ReactDOMComponent', () => { describe('Object stringification', function() { it('allows objects on known properties', function() { - var el = ReactTestUtils.renderIntoDocument( -
, - ); - expect(el.getAttribute('acceptCharset')).toBe('[object Object]'); + var el = ReactTestUtils.renderIntoDocument(
); + expect(el.getAttribute('accept-charset')).toBe('[object Object]'); }); it('should pass objects as attributes if they define toString', () => { @@ -2157,4 +2155,37 @@ describe('ReactDOMComponent', () => { expect(el.getAttribute('ajaxify')).toBe('ajaxy'); }); }); + + describe('String boolean attributes', function() { + it('does not assign string boolean attributes for custom attributes', function() { + spyOn(console, 'error'); + + var el = ReactTestUtils.renderIntoDocument(
); + + expect(el.hasAttribute('whatever')).toBe(false); + + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Invalid prop `whatever` on
tag', + ); + }); + + it('stringifies the boolean true for allowed attributes', function() { + var el = ReactTestUtils.renderIntoDocument(
); + + expect(el.getAttribute('spellCheck')).toBe('true'); + }); + + it('stringifies the boolean false for allowed attributes', function() { + var el = ReactTestUtils.renderIntoDocument(
); + + expect(el.getAttribute('spellCheck')).toBe('false'); + }); + + it('stringifies implicit booleans for allowed attributes', function() { + // eslint-disable-next-line react/jsx-boolean-value + var el = ReactTestUtils.renderIntoDocument(
); + + expect(el.getAttribute('spellCheck')).toBe('true'); + }); + }); }); From 041065115fa69b146a97d030695cc6d1842fc2e9 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 28 Aug 2017 23:36:32 -0400 Subject: [PATCH 05/36] Fix boolean tests, add boolean warning. --- .../dom/shared/DOMMarkupOperations.js | 3 ++- src/renderers/dom/shared/DOMProperty.js | 26 ++++++++++++++----- .../dom/shared/HTMLDOMPropertyConfig.js | 1 + .../__tests__/ReactDOMComponent-test.js | 6 ++--- .../ReactDOMServerIntegration-test.js | 24 ++++++++--------- .../hooks/ReactDOMUnknownPropertyHook.js | 14 ++++++++++ .../wrappers/__tests__/ReactDOMInput-test.js | 16 +++++++++--- 7 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/renderers/dom/shared/DOMMarkupOperations.js b/src/renderers/dom/shared/DOMMarkupOperations.js index 9323f7182776c..bc739bc6c6f31 100644 --- a/src/renderers/dom/shared/DOMMarkupOperations.js +++ b/src/renderers/dom/shared/DOMMarkupOperations.js @@ -99,8 +99,9 @@ var DOMMarkupOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { return attributeName + '=""'; + } else if (typeof value !== 'boolean' || DOMProperty.allowBoolean(name)) { + return attributeName + '=' + quoteAttributeValueForBrowser(value); } - return attributeName + '=' + quoteAttributeValueForBrowser(value); } else if (DOMProperty.shouldSetAttribute(name, value)) { if (value == null) { return ''; diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 5365b4bb4a75d..d517e05653568 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -217,13 +217,7 @@ var DOMProperty = { switch (typeof value) { case 'boolean': - if (propertyInfo) { - return ( - propertyInfo.hasBooleanValue || propertyInfo.hasStringBooleanValue - ); - } - var prefix = lowerCased.slice(0, 5); - return prefix === 'data-' || prefix === 'aria-'; + return DOMProperty.allowBoolean(name) case 'undefined': case 'number': case 'string': @@ -241,6 +235,24 @@ var DOMProperty = { : null; }, + allowBoolean(name) { + if (DOMProperty.isReservedProp(name)) { + return true; + } + + let propertyInfo = DOMProperty.getPropertyInfo(name) + + if (propertyInfo) { + return ( + propertyInfo.hasBooleanValue || propertyInfo.hasStringBooleanValue || propertyInfo.hasOverloadedBooleanValue + ); + } + + var prefix = name.toLowerCase().slice(0, 5); + + return prefix === 'data-' || prefix === 'aria-'; + }, + /** * Checks to see if a property name is within the list of properties * reserved for internal React operations. These properties should diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index be07c1804fa39..526120bf32f6c 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -43,6 +43,7 @@ var HTMLDOMPropertyConfig = { controls: HAS_BOOLEAN_VALUE, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, + defaultChecked: HAS_BOOLEAN_VALUE, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, draggable: HAS_STRING_BOOLEAN_VALUE, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 27bec59a6edc6..2d38c4dfa3d39 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2003,7 +2003,7 @@ describe('ReactDOMComponent', () => { expect(el.hasAttribute('whatever')).toBe(false); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Invalid prop `whatever` on
tag', + 'Warning: Received `true` for non-boolean attribute `whatever`' ); }); @@ -2016,7 +2016,7 @@ describe('ReactDOMComponent', () => { expect(el.hasAttribute('whatever')).toBe(false); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Invalid prop `whatever` on
tag', + 'Warning: Received `true` for non-boolean attribute `whatever`' ); }); @@ -2165,7 +2165,7 @@ describe('ReactDOMComponent', () => { expect(el.hasAttribute('whatever')).toBe(false); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Invalid prop `whatever` on
tag', + 'Warning: Received `true` for non-boolean attribute `whatever`.', ); }); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 2f5cd7034c2f3..b69bc5d462bae 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -597,15 +597,15 @@ describe('ReactDOMServerIntegration', () => { }); // this probably is just masking programmer error, but it is existing behavior. - itRenders('className prop with true value', async render => { - const e = await render(
); - expect(e.getAttribute('class')).toBe('true'); + itRenders('no className prop with true value', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('class')).toBe(false); }); // this probably is just masking programmer error, but it is existing behavior. - itRenders('className prop with false value', async render => { - const e = await render(
); - expect(e.getAttribute('class')).toBe('false'); + itRenders('no className prop with false value', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('class')).toBe(false); }); itRenders('no className prop with null value', async render => { @@ -664,15 +664,15 @@ describe('ReactDOMServerIntegration', () => { }); // this probably is just masking programmer error, but it is existing behavior. - itRenders('htmlFor prop with true value', async render => { - const e = await render(
); - expect(e.getAttribute('for')).toBe('true'); + itRenders('no htmlFor prop with true value', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('for')).toBe(false); }); // this probably is just masking programmer error, but it is existing behavior. - itRenders('htmlFor prop with false value', async render => { - const e = await render(
); - expect(e.getAttribute('for')).toBe('false'); + itRenders('no htmlFor prop with false value', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('for')).toBe(false); }); itRenders('no htmlFor prop with null value', async render => { diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 0864104d3e5d8..6d49ed9c0d429 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -116,6 +116,7 @@ if (__DEV__) { return true; } + // Known attributes should match the casing specified in the property config. if (possibleStandardNames.hasOwnProperty(lowerCasedName)) { var standardName = possibleStandardNames[lowerCasedName]; @@ -132,6 +133,19 @@ if (__DEV__) { } } + if (typeof value === 'boolean') { + warning( + DOMProperty.allowBoolean(name), + 'Received `%s` for non-boolean attribute `%s`. If this is expected, cast ' + + 'the value to a string.%s', + value, + name, + getStackAddendum(debugID), + ); + warnedProperties[name] = true; + return true; + } + // Now that we've validated casing, do not validate // data types for reserved props if (DOMProperty.isReservedProp(name)) { diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index b90923c772138..d9abd895619c5 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -301,20 +301,28 @@ describe('ReactDOMInput', () => { component.forceUpdate(); }); - it('should display "true" for `defaultValue` of `true`', () => { + it('should display nothing for `defaultValue` of `true`', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); var node = ReactDOM.findDOMNode(stub); - expect(node.value).toBe('true'); + expect(node.value).toBe(''); + expectDev(console.error.calls.count()).toBe(1); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( + 'Warning: Received `true` for non-boolean attribute `defaultValue`.' + ); }); - it('should display "false" for `defaultValue` of `false`', () => { + it('should display nothing for `defaultValue` of `false`', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); var node = ReactDOM.findDOMNode(stub); - expect(node.value).toBe('false'); + expect(node.value).toBe(''); + expectDev(console.error.calls.count()).toBe(1); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( + 'Warning: Received `false` for non-boolean attribute `defaultValue`.' + ); }); it('should update `defaultValue` for uncontrolled input', () => { From ab75d9a0561c05550b6cb62922b350e0375433fe Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 28 Aug 2017 23:58:50 -0400 Subject: [PATCH 06/36] Reserved props should allow booleans --- src/renderers/dom/shared/DOMProperty.js | 8 +++++--- .../dom/shared/HTMLDOMPropertyConfig.js | 4 ++-- .../shared/__tests__/ReactDOMComponent-test.js | 4 ++-- .../shared/hooks/ReactDOMUnknownPropertyHook.js | 1 - .../wrappers/__tests__/ReactDOMInput-test.js | 16 ++++------------ 5 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index d517e05653568..d92e11741f724 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -217,7 +217,7 @@ var DOMProperty = { switch (typeof value) { case 'boolean': - return DOMProperty.allowBoolean(name) + return DOMProperty.allowBoolean(name); case 'undefined': case 'number': case 'string': @@ -240,11 +240,13 @@ var DOMProperty = { return true; } - let propertyInfo = DOMProperty.getPropertyInfo(name) + let propertyInfo = DOMProperty.getPropertyInfo(name); if (propertyInfo) { return ( - propertyInfo.hasBooleanValue || propertyInfo.hasStringBooleanValue || propertyInfo.hasOverloadedBooleanValue + propertyInfo.hasBooleanValue || + propertyInfo.hasStringBooleanValue || + propertyInfo.hasOverloadedBooleanValue ); } diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 526120bf32f6c..19af1eadc0e8c 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -43,7 +43,6 @@ var HTMLDOMPropertyConfig = { controls: HAS_BOOLEAN_VALUE, default: HAS_BOOLEAN_VALUE, defer: HAS_BOOLEAN_VALUE, - defaultChecked: HAS_BOOLEAN_VALUE, disabled: HAS_BOOLEAN_VALUE, download: HAS_OVERLOADED_BOOLEAN_VALUE, draggable: HAS_STRING_BOOLEAN_VALUE, @@ -83,7 +82,8 @@ var HTMLDOMPropertyConfig = { htmlFor: 0, httpEquiv: 0, // Attributes with mutation methods must be specified in the whitelist - value: 0, + // Set the string boolean flag to allow the behavior + value: HAS_STRING_BOOLEAN_VALUE, }, DOMAttributeNames: { acceptCharset: 'accept-charset', diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 2d38c4dfa3d39..15d2326ef12f1 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2003,7 +2003,7 @@ describe('ReactDOMComponent', () => { expect(el.hasAttribute('whatever')).toBe(false); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Received `true` for non-boolean attribute `whatever`' + 'Warning: Received `true` for non-boolean attribute `whatever`', ); }); @@ -2016,7 +2016,7 @@ describe('ReactDOMComponent', () => { expect(el.hasAttribute('whatever')).toBe(false); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Received `true` for non-boolean attribute `whatever`' + 'Warning: Received `true` for non-boolean attribute `whatever`', ); }); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 6d49ed9c0d429..6b728982a655e 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -116,7 +116,6 @@ if (__DEV__) { return true; } - // Known attributes should match the casing specified in the property config. if (possibleStandardNames.hasOwnProperty(lowerCasedName)) { var standardName = possibleStandardNames[lowerCasedName]; diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index d9abd895619c5..b90923c772138 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -301,28 +301,20 @@ describe('ReactDOMInput', () => { component.forceUpdate(); }); - it('should display nothing for `defaultValue` of `true`', () => { + it('should display "true" for `defaultValue` of `true`', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); var node = ReactDOM.findDOMNode(stub); - expect(node.value).toBe(''); - expectDev(console.error.calls.count()).toBe(1); - expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( - 'Warning: Received `true` for non-boolean attribute `defaultValue`.' - ); + expect(node.value).toBe('true'); }); - it('should display nothing for `defaultValue` of `false`', () => { + it('should display "false" for `defaultValue` of `false`', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); var node = ReactDOM.findDOMNode(stub); - expect(node.value).toBe(''); - expectDev(console.error.calls.count()).toBe(1); - expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( - 'Warning: Received `false` for non-boolean attribute `defaultValue`.' - ); + expect(node.value).toBe('false'); }); it('should update `defaultValue` for uncontrolled input', () => { From 6077e0b7bf103df8568153d7c460b46f56038646 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 02:08:14 -0700 Subject: [PATCH 07/36] Remove outdated comments --- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index b69bc5d462bae..0bab757ca6505 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -596,13 +596,11 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('class')).toBe(''); }); - // this probably is just masking programmer error, but it is existing behavior. itRenders('no className prop with true value', async render => { const e = await render(
, 1); expect(e.hasAttribute('class')).toBe(false); }); - // this probably is just masking programmer error, but it is existing behavior. itRenders('no className prop with false value', async render => { const e = await render(
, 1); expect(e.hasAttribute('class')).toBe(false); @@ -663,13 +661,11 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('for')).toBe(''); }); - // this probably is just masking programmer error, but it is existing behavior. itRenders('no htmlFor prop with true value', async render => { const e = await render(
, 1); expect(e.hasAttribute('for')).toBe(false); }); - // this probably is just masking programmer error, but it is existing behavior. itRenders('no htmlFor prop with false value', async render => { const e = await render(
, 1); expect(e.hasAttribute('for')).toBe(false); From 6f913ed0d87af726b5c50dabf1d3f24ec249f9b8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 02:11:50 -0700 Subject: [PATCH 08/36] Style tweaks --- src/renderers/dom/shared/DOMMarkupOperations.js | 2 +- src/renderers/dom/shared/DOMProperty.js | 14 ++------------ .../shared/hooks/ReactDOMUnknownPropertyHook.js | 2 +- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/renderers/dom/shared/DOMMarkupOperations.js b/src/renderers/dom/shared/DOMMarkupOperations.js index bc739bc6c6f31..85dbfccb8d911 100644 --- a/src/renderers/dom/shared/DOMMarkupOperations.js +++ b/src/renderers/dom/shared/DOMMarkupOperations.js @@ -99,7 +99,7 @@ var DOMMarkupOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { return attributeName + '=""'; - } else if (typeof value !== 'boolean' || DOMProperty.allowBoolean(name)) { + } else if (typeof value !== 'boolean' || DOMProperty.shouldAttributeAcceptBooleanValue(name)) { return attributeName + '=' + quoteAttributeValueForBrowser(value); } } else if (DOMProperty.shouldSetAttribute(name, value)) { diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index d92e11741f724..11fd538927f94 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -206,18 +206,12 @@ var DOMProperty = { if (DOMProperty.isReservedProp(name)) { return false; } - if (value === null) { return true; } - - var lowerCased = name.toLowerCase(); - - var propertyInfo = DOMProperty.properties[name]; - switch (typeof value) { case 'boolean': - return DOMProperty.allowBoolean(name); + return DOMProperty.shouldAttributeAcceptBooleanValue(name); case 'undefined': case 'number': case 'string': @@ -235,13 +229,11 @@ var DOMProperty = { : null; }, - allowBoolean(name) { + shouldAttributeAcceptBooleanValue(name) { if (DOMProperty.isReservedProp(name)) { return true; } - let propertyInfo = DOMProperty.getPropertyInfo(name); - if (propertyInfo) { return ( propertyInfo.hasBooleanValue || @@ -249,9 +241,7 @@ var DOMProperty = { propertyInfo.hasOverloadedBooleanValue ); } - var prefix = name.toLowerCase().slice(0, 5); - return prefix === 'data-' || prefix === 'aria-'; }, diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 6b728982a655e..7677f8ba1f2e6 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -134,7 +134,7 @@ if (__DEV__) { if (typeof value === 'boolean') { warning( - DOMProperty.allowBoolean(name), + DOMProperty.shouldAttributeAcceptBooleanValue(name), 'Received `%s` for non-boolean attribute `%s`. If this is expected, cast ' + 'the value to a string.%s', value, From aca8d9c27e7138e06c3af75a0d6c0eb8f471b460 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 03:22:48 -0700 Subject: [PATCH 09/36] Don't treat dashed SVG tags as custom elements --- fixtures/packaging/babel-standalone/dev.html | 6 ++--- .../dom/fiber/ReactDOMFiberComponent.js | 21 ++++++++++++------ .../dom/shared/DOMMarkupOperations.js | 5 ++++- .../ReactDOMServerIntegration-test.js | 9 ++++++++ .../dom/stack/client/ReactDOMComponent.js | 22 ++++++++++++++----- .../shared/server/ReactPartialRenderer.js | 7 +++++- 6 files changed, 52 insertions(+), 18 deletions(-) diff --git a/fixtures/packaging/babel-standalone/dev.html b/fixtures/packaging/babel-standalone/dev.html index 9c3a931badcc4..1bf91ddc422eb 100644 --- a/fixtures/packaging/babel-standalone/dev.html +++ b/fixtures/packaging/babel-standalone/dev.html @@ -1,12 +1,12 @@ - - + +
diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 0ce5956146a38..d118052e2bff0 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -305,11 +305,10 @@ var ReactDOMFiberComponent = { if (namespaceURI === HTML_NAMESPACE) { namespaceURI = getIntrinsicNamespace(type); } - if (__DEV__) { - var isCustomComponentTag = isCustomComponent(type, props); - } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { + var isCustomComponentTag = + isCustomComponent(type, props) && namespaceURI === HTML_NAMESPACE; // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -370,7 +369,9 @@ var ReactDOMFiberComponent = { rawProps: Object, rootContainerElement: Element | Document, ): void { - var isCustomComponentTag = isCustomComponent(tag, rawProps); + var isCustomComponentTag = + isCustomComponent(tag, rawProps) && + domElement.namespaceURI === HTML_NAMESPACE; if (__DEV__) { validatePropertiesInDevelopment(tag, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { @@ -740,8 +741,12 @@ var ReactDOMFiberComponent = { lastRawProps: Object, nextRawProps: Object, ): void { - var wasCustomComponentTag = isCustomComponent(tag, lastRawProps); - var isCustomComponentTag = isCustomComponent(tag, nextRawProps); + var wasCustomComponentTag = + isCustomComponent(tag, lastRawProps) && + domElement.namespaceURI === HTML_NAMESPACE; + var isCustomComponentTag = + isCustomComponent(tag, nextRawProps) && + domElement.namespaceURI === HTML_NAMESPACE; // Apply the diff. updateDOMProperties( domElement, @@ -781,7 +786,9 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - var isCustomComponentTag = isCustomComponent(tag, rawProps); + var isCustomComponentTag = + isCustomComponent(tag, rawProps) && + domElement.namespaceURI === HTML_NAMESPACE; validatePropertiesInDevelopment(tag, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( diff --git a/src/renderers/dom/shared/DOMMarkupOperations.js b/src/renderers/dom/shared/DOMMarkupOperations.js index 85dbfccb8d911..772133625bf20 100644 --- a/src/renderers/dom/shared/DOMMarkupOperations.js +++ b/src/renderers/dom/shared/DOMMarkupOperations.js @@ -99,7 +99,10 @@ var DOMMarkupOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { return attributeName + '=""'; - } else if (typeof value !== 'boolean' || DOMProperty.shouldAttributeAcceptBooleanValue(name)) { + } else if ( + typeof value !== 'boolean' || + DOMProperty.shouldAttributeAcceptBooleanValue(name) + ) { return attributeName + '=' + quoteAttributeValueForBrowser(value); } } else if (DOMProperty.shouldSetAttribute(name, value)) { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 0bab757ca6505..7a191488ada8d 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -950,6 +950,15 @@ describe('ReactDOMServerIntegration', () => { }, ); + itRenders( + 'known SVG attributes for elements with dashes in tag', + async render => { + const e = await render(); + expect(e.firstChild.hasAttribute('accentHeight')).toBe(false); + expect(e.firstChild.getAttribute('accent-height')).toBe('10'); + }, + ); + itRenders('cased custom attributes', async render => { const e = await render(
); expect(e.getAttribute('fooBar')).toBe('test'); diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 8f20ebb891393..fd2cdcc005f17 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -480,9 +480,6 @@ ReactDOMComponent.Mixin = { assertValidProps(this, props); - if (__DEV__) { - var isCustomComponentTag = isCustomComponent(this._tag, props); - } // We create tags in the namespace of their parent container, except HTML // tags get no namespace. var namespaceURI; @@ -500,6 +497,10 @@ ReactDOMComponent.Mixin = { ) { namespaceURI = Namespaces.html; } + if (__DEV__) { + var isCustomComponentTag = + isCustomComponent(this._tag, props) && namespaceURI === Namespaces.html; + } if (namespaceURI === Namespaces.html) { if (__DEV__) { warning( @@ -696,7 +697,11 @@ ReactDOMComponent.Mixin = { propValue = createMarkupForStyles(propValue, this); } var markup = null; - if (this._tag != null && isCustomComponent(this._tag, props)) { + if ( + this._tag != null && + isCustomComponent(this._tag, props) && + this._namespaceURI === Namespaces.html + ) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -879,7 +884,9 @@ ReactDOMComponent.Mixin = { } assertValidProps(this, nextProps); - var isCustomComponentTag = isCustomComponent(this._tag, nextProps); + var isCustomComponentTag = + isCustomComponent(this._tag, nextProps) && + this._namespaceURI === Namespaces.html; this._updateDOMProperties( lastProps, nextProps, @@ -953,7 +960,10 @@ ReactDOMComponent.Mixin = { } else if (registrationNameModules.hasOwnProperty(propKey)) { // Do nothing for event names. } else if (!DOMProperty.isReservedProp(propKey)) { - if (isCustomComponent(this._tag, lastProps)) { + if ( + isCustomComponent(this._tag, lastProps) && + this._namespaceURI === Namespaces.html + ) { DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey); } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 64cdb8109b9a5..9e82350a92ea8 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -262,6 +262,7 @@ function createOpenTagMarkup( tagVerbatim, tagLowercase, props, + namespace, makeStaticMarkup, isRootElement, instForDebug, @@ -280,7 +281,10 @@ function createOpenTagMarkup( propValue = createMarkupForStyles(propValue, instForDebug); } var markup = null; - if (isCustomComponent(tagLowercase, props)) { + if ( + isCustomComponent(tagLowercase, props) && + namespace === Namespaces.html + ) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -786,6 +790,7 @@ class ReactDOMServerRenderer { element.type, tag, props, + namespace, this.makeStaticMarkup, this.stack.length === 1, null, From af7d035f1fadc9eaaa73036b8135dbf35c8460d5 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 29 Aug 2017 08:13:22 -0400 Subject: [PATCH 10/36] SVG elements like font-face are not custom attributes - Adds exceptions to isCustomAttribute for dashed SVG elements - Use consistent custom element check across all modules --- .../__tests__/ReactDOMComponent-test.js | 30 +++++++++++++++++++ .../shared/hooks/ReactDOMInvalidARIAHook.js | 3 +- .../hooks/ReactDOMUnknownPropertyHook.js | 3 +- .../dom/shared/utils/isCustomComponent.js | 15 ++++++++++ .../shared/server/ReactPartialRenderer.js | 5 +--- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 15d2326ef12f1..3acaec64dc640 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2188,4 +2188,34 @@ describe('ReactDOMComponent', () => { expect(el.getAttribute('spellCheck')).toBe('true'); }); }); + + describe('Hyphenated SVG elements', function() { + it('the font-face element is not a custom element', function() { + spyOn(console, 'error'); + var el = ReactTestUtils.renderIntoDocument( + , + ); + + expect(el.hasAttribute('x-height')).toBe(false); + + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`', + ); + }); + + it('the font-face element does not allow unknown boolean values', function() { + spyOn(console, 'error'); + var el = ReactTestUtils.renderIntoDocument( + , + ); + + expect(el.hasAttribute('whatever')).toBe(false); + + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Received `false` for non-boolean attribute `whatever`.', + ); + }); + }); }); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index 7ae5f553bca9f..705c5aa207d20 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -12,6 +12,7 @@ 'use strict'; var DOMProperty = require('DOMProperty'); +var isCustomComponent = require('isCustomComponent'); var warnedProperties = {}; var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$'); @@ -145,7 +146,7 @@ function warnInvalidARIAProps(type, props, debugID) { } function validateProperties(type, props, debugID /* Stack only */) { - if (type.indexOf('-') >= 0 || props.is) { + if (isCustomComponent(type, props)) { return; } warnInvalidARIAProps(type, props, debugID); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 7677f8ba1f2e6..58d9f3ca6c3a9 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -13,6 +13,7 @@ var DOMProperty = require('DOMProperty'); var EventPluginRegistry = require('EventPluginRegistry'); +var isCustomComponent = require('isCustomComponent'); if (__DEV__) { var warning = require('fbjs/lib/warning'); @@ -208,7 +209,7 @@ var warnUnknownProperties = function(type, props, debugID) { }; function validateProperties(type, props, debugID /* Stack only */) { - if (type.indexOf('-') >= 0 || props.is) { + if (isCustomComponent(type, props)) { return; } warnUnknownProperties(type, props, debugID); diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index 2563a1e2b5a13..96285beeee36c 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -11,7 +11,22 @@ 'use strict'; +// https://www.w3.org/TR/SVG/eltindex.html +var DashedSVGElements = { + 'color-profile': true, + 'font-face': true, + 'font-face-format': true, + 'font-face-name': true, + 'font-face-src': true, + 'font-face-uri': true, + 'missing-glyph': true, +}; + function isCustomComponent(tagName, props) { + if (DashedSVGElements[tagName]) { + return false; + } + return tagName.indexOf('-') >= 0 || props.is != null; } diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 9e82350a92ea8..5131e23676200 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -28,6 +28,7 @@ var hyphenateStyleName = require('fbjs/lib/hyphenateStyleName'); var invariant = require('fbjs/lib/invariant'); var memoizeStringOnly = require('fbjs/lib/memoizeStringOnly'); var omittedCloseTags = require('omittedCloseTags'); +var isCustomComponent = require('isCustomComponent'); var toArray = React.Children.toArray; @@ -254,10 +255,6 @@ var RESERVED_PROPS = { suppressContentEditableWarning: null, }; -function isCustomComponent(tagName, props) { - return tagName.indexOf('-') >= 0 || props.is != null; -} - function createOpenTagMarkup( tagVerbatim, tagLowercase, From 9c0751fc356e7e797815e5dd18df01ee85764436 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 29 Aug 2017 09:46:14 -0400 Subject: [PATCH 11/36] Move namespace check to isCustomAttribute. Add caveat for stack. --- .../dom/fiber/ReactDOMFiberComponent.js | 57 +++++++++++-------- .../__tests__/ReactDOMComponent-test.js | 52 +++++++++-------- .../shared/hooks/ReactDOMInvalidARIAHook.js | 8 +-- .../hooks/ReactDOMUnknownPropertyHook.js | 9 +-- .../dom/shared/utils/isCustomComponent.js | 22 +++---- .../dom/stack/client/ReactDOMComponent.js | 23 ++++---- .../shared/server/ReactPartialRenderer.js | 13 ++--- 7 files changed, 97 insertions(+), 87 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index d118052e2bff0..fdf3f519a00bd 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -40,10 +40,10 @@ if (__DEV__) { var ReactDOMUnknownPropertyHook = require('ReactDOMUnknownPropertyHook'); var {validateProperties: validateARIAProperties} = ReactDOMInvalidARIAHook; var { - validateProperties: validateInputPropertes, + validateProperties: validateInputProperties, } = ReactDOMNullInputValuePropHook; var { - validateProperties: validateUnknownPropertes, + validateProperties: validateUnknownProperties, } = ReactDOMUnknownPropertyHook; } @@ -70,10 +70,10 @@ if (__DEV__) { time: true, }; - var validatePropertiesInDevelopment = function(type, props) { - validateARIAProperties(type, props); - validateInputPropertes(type, props); - validateUnknownPropertes(type, props); + var validatePropertiesInDevelopment = function(type, namespace, props) { + validateARIAProperties(type, props, namespace); + validateInputProperties(type, props); + validateUnknownProperties(type, props, namespace); }; var warnForTextDifference = function(serverText: string, clientText: string) { @@ -307,8 +307,7 @@ var ReactDOMFiberComponent = { } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { - var isCustomComponentTag = - isCustomComponent(type, props) && namespaceURI === HTML_NAMESPACE; + var isCustomComponentTag = isCustomComponent(type, props, namespaceURI); // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -369,11 +368,13 @@ var ReactDOMFiberComponent = { rawProps: Object, rootContainerElement: Element | Document, ): void { - var isCustomComponentTag = - isCustomComponent(tag, rawProps) && - domElement.namespaceURI === HTML_NAMESPACE; + var isCustomComponentTag = isCustomComponent( + tag, + rawProps, + domElement.namespaceURI, + ); if (__DEV__) { - validatePropertiesInDevelopment(tag, rawProps); + validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, @@ -544,7 +545,11 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - validatePropertiesInDevelopment(tag, nextRawProps); + validatePropertiesInDevelopment( + tag, + domElement.namespaceURI, + nextRawProps, + ); } var updatePayload: null | Array = null; @@ -741,12 +746,16 @@ var ReactDOMFiberComponent = { lastRawProps: Object, nextRawProps: Object, ): void { - var wasCustomComponentTag = - isCustomComponent(tag, lastRawProps) && - domElement.namespaceURI === HTML_NAMESPACE; - var isCustomComponentTag = - isCustomComponent(tag, nextRawProps) && - domElement.namespaceURI === HTML_NAMESPACE; + var wasCustomComponentTag = isCustomComponent( + tag, + lastRawProps, + domElement.namespaceURI, + ); + var isCustomComponentTag = isCustomComponent( + tag, + nextRawProps, + domElement.namespaceURI, + ); // Apply the diff. updateDOMProperties( domElement, @@ -786,10 +795,12 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - var isCustomComponentTag = - isCustomComponent(tag, rawProps) && - domElement.namespaceURI === HTML_NAMESPACE; - validatePropertiesInDevelopment(tag, rawProps); + var isCustomComponentTag = isCustomComponent( + tag, + rawProps, + domElement.namespaceURI, + ); + validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 3acaec64dc640..050c31317b5e0 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2189,33 +2189,39 @@ describe('ReactDOMComponent', () => { }); }); - describe('Hyphenated SVG elements', function() { - it('the font-face element is not a custom element', function() { - spyOn(console, 'error'); - var el = ReactTestUtils.renderIntoDocument( - , - ); + if (ReactDOMFeatureFlags.useFiber) { + describe('Hyphenated SVG elements', function() { + it('the font-face element is not a custom element', function() { + spyOn(console, 'error'); + var el = ReactTestUtils.renderIntoDocument( + , + ); - expect(el.hasAttribute('x-height')).toBe(false); + expect(el.querySelector('font-face').hasAttribute('x-height')).toBe( + false, + ); - expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`', - ); - }); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`', + ); + }); - it('the font-face element does not allow unknown boolean values', function() { - spyOn(console, 'error'); - var el = ReactTestUtils.renderIntoDocument( - , - ); + it('the font-face element does not allow unknown boolean values', function() { + spyOn(console, 'error'); + var el = ReactTestUtils.renderIntoDocument( + , + ); - expect(el.hasAttribute('whatever')).toBe(false); + expect(el.querySelector('font-face').hasAttribute('whatever')).toBe( + false, + ); - expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Received `false` for non-boolean attribute `whatever`.', - ); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Received `false` for non-boolean attribute `whatever`.', + ); + }); }); - }); + } }); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index 705c5aa207d20..ca0251121a551 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -145,8 +145,8 @@ function warnInvalidARIAProps(type, props, debugID) { } } -function validateProperties(type, props, debugID /* Stack only */) { - if (isCustomComponent(type, props)) { +function validateProperties(type, props, namespace, debugID /* Stack only */) { + if (isCustomComponent(type, props, namespace)) { return; } warnInvalidARIAProps(type, props, debugID); @@ -158,12 +158,12 @@ var ReactDOMInvalidARIAHook = { // Stack onBeforeMountComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, debugID); + validateProperties(element.type, element.props, null, debugID); } }, onBeforeUpdateComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, debugID); + validateProperties(element.type, element.props, null, debugID); } }, }; diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 58d9f3ca6c3a9..80862310de9a5 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -14,6 +14,7 @@ var DOMProperty = require('DOMProperty'); var EventPluginRegistry = require('EventPluginRegistry'); var isCustomComponent = require('isCustomComponent'); +var DOMNamespaces = require('DOMNamespaces'); if (__DEV__) { var warning = require('fbjs/lib/warning'); @@ -208,8 +209,8 @@ var warnUnknownProperties = function(type, props, debugID) { } }; -function validateProperties(type, props, debugID /* Stack only */) { - if (isCustomComponent(type, props)) { +function validateProperties(type, props, namespace, debugID /* Stack only */) { + if (isCustomComponent(type, props, namespace)) { return; } warnUnknownProperties(type, props, debugID); @@ -221,12 +222,12 @@ var ReactDOMUnknownPropertyHook = { // Stack onBeforeMountComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, debugID); + validateProperties(element.type, element.props, null, debugID); } }, onBeforeUpdateComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, debugID); + validateProperties(element.type, element.props, null, debugID); } }, }; diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index 96285beeee36c..f12b50af4e1ee 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -11,23 +11,17 @@ 'use strict'; -// https://www.w3.org/TR/SVG/eltindex.html -var DashedSVGElements = { - 'color-profile': true, - 'font-face': true, - 'font-face-format': true, - 'font-face-name': true, - 'font-face-src': true, - 'font-face-uri': true, - 'missing-glyph': true, -}; +var DOMNamespaces = require('DOMNamespaces'); +var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; -function isCustomComponent(tagName, props) { - if (DashedSVGElements[tagName]) { - return false; +function isCustomComponent(tagName, props, namespace) { + if (tagName.indexOf('-') >= 0 || props.is != null) { + // TODO: We always have a namespace with fiber. Drop the first + // check when Stack is removed. + return namespace == null || namespace === HTML_NAMESPACE; } - return tagName.indexOf('-') >= 0 || props.is != null; + return false; } module.exports = isCustomComponent; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index fd2cdcc005f17..85a2c21d5f8cc 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -498,8 +498,11 @@ ReactDOMComponent.Mixin = { namespaceURI = Namespaces.html; } if (__DEV__) { - var isCustomComponentTag = - isCustomComponent(this._tag, props) && namespaceURI === Namespaces.html; + var isCustomComponentTag = isCustomComponent( + this._tag, + props, + namespaceURI, + ); } if (namespaceURI === Namespaces.html) { if (__DEV__) { @@ -699,8 +702,7 @@ ReactDOMComponent.Mixin = { var markup = null; if ( this._tag != null && - isCustomComponent(this._tag, props) && - this._namespaceURI === Namespaces.html + isCustomComponent(this._tag, props, this._namespaceURI) ) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( @@ -884,9 +886,11 @@ ReactDOMComponent.Mixin = { } assertValidProps(this, nextProps); - var isCustomComponentTag = - isCustomComponent(this._tag, nextProps) && - this._namespaceURI === Namespaces.html; + var isCustomComponentTag = isCustomComponent( + this._tag, + nextProps, + this._namespaceURI, + ); this._updateDOMProperties( lastProps, nextProps, @@ -960,10 +964,7 @@ ReactDOMComponent.Mixin = { } else if (registrationNameModules.hasOwnProperty(propKey)) { // Do nothing for event names. } else if (!DOMProperty.isReservedProp(propKey)) { - if ( - isCustomComponent(this._tag, lastProps) && - this._namespaceURI === Namespaces.html - ) { + if (isCustomComponent(this._tag, lastProps, this._namespaceURI)) { DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey); } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 5131e23676200..232aa0a67dd9a 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -45,10 +45,10 @@ if (__DEV__) { var { validateProperties: validateUnknownPropertes, } = require('ReactDOMUnknownPropertyHook'); - var validatePropertiesInDevelopment = function(type, props) { - validateARIAProperties(type, props); + var validatePropertiesInDevelopment = function(type, namespace, props) { + validateARIAProperties(type, namespace, props); validateInputPropertes(type, props); - validateUnknownPropertes(type, props); + validateUnknownPropertes(type, namespace, props); }; var describeComponentFrame = require('describeComponentFrame'); @@ -278,10 +278,7 @@ function createOpenTagMarkup( propValue = createMarkupForStyles(propValue, instForDebug); } var markup = null; - if ( - isCustomComponent(tagLowercase, props) && - namespace === Namespaces.html - ) { + if (isCustomComponent(tagLowercase, props, namespace)) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -778,7 +775,7 @@ class ReactDOMServerRenderer { } if (__DEV__) { - validatePropertiesInDevelopment(tag, props); + validatePropertiesInDevelopment(tag, props, namespace); } assertValidProps(tag, props); From f8da44ef3abae83541663ba64455d6f4bbebbfd4 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 29 Aug 2017 10:11:53 -0400 Subject: [PATCH 12/36] Remove unused namespace variable assignment --- src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 80862310de9a5..574969b6a9a7c 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -14,7 +14,6 @@ var DOMProperty = require('DOMProperty'); var EventPluginRegistry = require('EventPluginRegistry'); var isCustomComponent = require('isCustomComponent'); -var DOMNamespaces = require('DOMNamespaces'); if (__DEV__) { var warning = require('fbjs/lib/warning'); From b93e093a346f49ac22464a07e786806e5e3c54bb Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 09:07:20 -0700 Subject: [PATCH 13/36] Fix the DEV-only whitelist --- src/renderers/dom/shared/hooks/possibleStandardNames.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/hooks/possibleStandardNames.js b/src/renderers/dom/shared/hooks/possibleStandardNames.js index 2d0a81771ece6..57b6cdb1e908e 100644 --- a/src/renderers/dom/shared/hooks/possibleStandardNames.js +++ b/src/renderers/dom/shared/hooks/possibleStandardNames.js @@ -64,7 +64,7 @@ var possibleStandardNames = { enctype: 'encType', for: 'htmlFor', form: 'form', - formMethod: 'formMethod', + formmethod: 'formMethod', formaction: 'formAction', formenctype: 'formEncType', formnovalidate: 'formNoValidate', @@ -99,7 +99,7 @@ var possibleStandardNames = { loop: 'loop', low: 'low', manifest: 'manifest', - marginWidth: 'marginWidth', + marginwidth: 'marginWidth', marginheight: 'marginHeight', max: 'max', maxlength: 'maxLength', @@ -107,7 +107,7 @@ var possibleStandardNames = { mediagroup: 'mediaGroup', method: 'method', min: 'min', - minLength: 'minlength', + minlength: 'minLength', multiple: 'multiple', muted: 'muted', name: 'name', @@ -236,7 +236,7 @@ var possibleStandardNames = { filter: 'filter', filterres: 'filterRes', filterunits: 'filterUnits', - floodOpacity: 'floodOpacity', + floodopacity: 'floodOpacity', 'flood-opacity': 'floodOpacity', floodcolor: 'floodColor', 'flood-color': 'floodColor', From fbcced12347ec01cc8367927e9175bae21fd3936 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 09:32:14 -0700 Subject: [PATCH 14/36] Don't read property twice --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index fdf3f519a00bd..fd10cf0e99995 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -746,15 +746,16 @@ var ReactDOMFiberComponent = { lastRawProps: Object, nextRawProps: Object, ): void { + var namespaceURI = domElement.namespaceURI; var wasCustomComponentTag = isCustomComponent( tag, lastRawProps, - domElement.namespaceURI, + namespaceURI, ); var isCustomComponentTag = isCustomComponent( tag, nextRawProps, - domElement.namespaceURI, + namespaceURI, ); // Apply the diff. updateDOMProperties( From a270e03bdf3ef57292f1666b5b5e89d5f3932132 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 09:48:34 -0700 Subject: [PATCH 15/36] Ignore and warn about non-string `is` attribute --- .../dom/fiber/ReactDOMFiberComponent.js | 2 +- .../shared/__tests__/ReactDOMComponent-test.js | 14 +++++++++++++- .../shared/hooks/ReactDOMUnknownPropertyHook.js | 17 +++++++++++++++++ .../dom/shared/utils/isCustomComponent.js | 2 +- .../dom/stack/client/ReactDOMComponent.js | 2 +- 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index fd10cf0e99995..b1a7bc1919f86 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -326,7 +326,7 @@ var ReactDOMFiberComponent = { // This is guaranteed to yield a script element. var firstChild = ((div.firstChild: any): HTMLScriptElement); domElement = div.removeChild(firstChild); - } else if (props.is) { + } else if (typeof props.is === 'string') { // $FlowIssue `createElement` should be updated for Web Components domElement = ownerDocument.createElement(type, {is: props.is}); } else { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 050c31317b5e0..13f1fd2e00ba5 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -635,12 +635,24 @@ describe('ReactDOMComponent', () => { expect(nodeValueSetter.mock.calls.length).toBe(3); }); - it('should ignore attribute whitelist for elements with the "is: attribute', () => { + it('should ignore attribute whitelist for elements with the "is" attribute', () => { var container = document.createElement('div'); ReactDOM.render(, container); expect(container.firstChild.hasAttribute('cowabunga')).toBe(true); }); + it('should warn about non-string "is" attribute', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(, container); + + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Received a `function` for string attribute `is`. If this is expected, cast ' + + 'the value to a string.', + ); + }); + it('should not update when switching between null/undefined', () => { var container = document.createElement('div'); var node = ReactDOM.render(
, container); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 574969b6a9a7c..099b615dd56c9 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -105,6 +105,23 @@ if (__DEV__) { return true; } + if ( + lowerCasedName === 'is' && + value !== null && + value !== undefined && + typeof value !== 'string' + ) { + warning( + false, + 'Received a `%s` for string attribute `is`. If this is expected, cast ' + + 'the value to a string.%s', + typeof value, + getStackAddendum(debugID), + ); + warnedProperties[name] = true; + return true; + } + if (typeof value === 'number' && isNaN(value)) { warning( false, diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index f12b50af4e1ee..05156574cd068 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -15,7 +15,7 @@ var DOMNamespaces = require('DOMNamespaces'); var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; function isCustomComponent(tagName, props, namespace) { - if (tagName.indexOf('-') >= 0 || props.is != null) { + if (tagName.indexOf('-') >= 0 || typeof props.is === 'string') { // TODO: We always have a namespace with fiber. Drop the first // check when Stack is removed. return namespace == null || namespace === HTML_NAMESPACE; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 85a2c21d5f8cc..5f7a5a1e9b5da 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -552,7 +552,7 @@ ReactDOMComponent.Mixin = { var div = ownerDocument.createElement('div'); div.innerHTML = `<${type}>`; el = div.removeChild(div.firstChild); - } else if (props.is) { + } else if (typeof props.is === 'string') { el = ownerDocument.createElement(type, {is: props.is}); } else { // Separate else branch instead of using `props.is || undefined` above because of a Firefox bug. From ed92af01a80bc863b76430cf72d04b21d1bd0378 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 10:08:47 -0700 Subject: [PATCH 16/36] Blacklist "aria" and "data" attributes --- src/renderers/dom/shared/DOMProperty.js | 2 ++ .../__tests__/ReactDOMComponent-test.js | 14 +++++++++++++ .../ReactDOMServerIntegration-test.js | 10 ++++++++++ .../hooks/ReactDOMUnknownPropertyHook.js | 20 +++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 11fd538927f94..eab32315414b7 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -16,7 +16,9 @@ var invariant = require('fbjs/lib/invariant'); // These attributes should be all lowercase to allow for // case insensitive checks var RESERVED_PROPS = { + aria: true, children: true, + data: true, dangerouslysetinnerhtml: true, autofocus: true, defaultvalue: true, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 13f1fd2e00ba5..b4f478f87f550 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -827,6 +827,20 @@ describe('ReactDOMComponent', () => { ); }); + it('should warn on props reserved for future use', () => { + spyOn(console, 'error'); + ReactTestUtils.renderIntoDocument(
); + expectDev(console.error.calls.count()).toBe(2); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'The `data` attribute is reserved for future use in React, and will be ignored. ' + + 'Pass individual `data-` attributes instead.', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'The `aria` attribute is reserved for future use in React, and will be ignored. ' + + 'Pass individual `aria-` attributes instead.', + ); + }); + it('should warn if the tag is unrecognized', () => { spyOn(console, 'error'); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 7a191488ada8d..3eb0bc8881a8b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -845,6 +845,11 @@ describe('ReactDOMServerIntegration', () => { const e = await render(
); expect(e.hasAttribute('aria-label')).toBe(false); }); + + itRenders('no "aria" attribute', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('aria')).toBe(false); + }); }); describe('cased attributes', function() { @@ -888,6 +893,11 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('data-foo')).toBe('bar'); }); + itRenders('no "data" attribute', async render => { + const e = await render(
, 1); + expect(e.hasAttribute('data')).toBe(false); + }); + itRenders('no unknown data- attributes with null value', async render => { const e = await render(
); expect(e.hasAttribute('data-foo')).toBe(false); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 099b615dd56c9..c5c305a2c20be 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -105,6 +105,26 @@ if (__DEV__) { return true; } + if (lowerCasedName === 'aria') { + warning( + false, + 'The `aria` attribute is reserved for future use in React, and will be ignored. ' + + 'Pass individual `aria-` attributes instead.', + ); + warnedProperties[name] = true; + return true; + } + + if (lowerCasedName === 'data') { + warning( + false, + 'The `data` attribute is reserved for future use in React, and will be ignored. ' + + 'Pass individual `data-` attributes instead.', + ); + warnedProperties[name] = true; + return true; + } + if ( lowerCasedName === 'is' && value !== null && From 5a339a1a3f6f9abf2fdc77e32cfc3f11f3dbe90a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 10:19:33 -0700 Subject: [PATCH 17/36] Don't pass unknown on* attributes through --- src/renderers/dom/shared/DOMProperty.js | 6 ++++++ .../shared/__tests__/ReactDOMServerIntegration-test.js | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index eab32315414b7..fbf4f05e27877 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -211,6 +211,12 @@ var DOMProperty = { if (value === null) { return true; } + if ( + (name[0] === 'o' || name[0] === 'O') && + (name[1] === 'n' || name[1] === 'N') + ) { + return false; + } switch (typeof value) { case 'boolean': return DOMProperty.shouldAttributeAcceptBooleanValue(name); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 3eb0bc8881a8b..b81834404f1a4 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -981,6 +981,14 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('onClick')).toBe(null); expect(e.getAttribute('click')).toBe(null); }); + + itRenders('no unknown events', async render => { + const e = await render( +
, + 1, + ); + expect(e.getAttribute('onunknownevent')).toBe(null); + }); }); describe('elements and children', function() { From 0b2ba65191b7e3c441f906dd5c432353e24cbe99 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 10:29:21 -0700 Subject: [PATCH 18/36] Remove dead code --- .../dom/shared/hooks/ReactDOMUnknownPropertyHook.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index c5c305a2c20be..50931df586e22 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -205,23 +205,10 @@ var warnUnknownProperties = function(type, props, debugID) { var isValid = validateProperty(type, key, props[key], debugID); if (!isValid) { unknownProps.push(key); - var value = props[key]; - if (typeof value === 'object' && value !== null) { - warning( - false, - 'The %s prop on <%s> is not a known property, and was given an object.' + - 'Remove it, or it will appear in the ' + - 'DOM after a future React update.%s', - key, - type, - getStackAddendum(debugID), - ); - } } } var unknownPropString = unknownProps.map(prop => '`' + prop + '`').join(', '); - if (unknownProps.length === 1) { warning( false, From 3f853165a877183b2d906d9f17b67cfeac76def2 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 29 Aug 2017 13:36:56 -0400 Subject: [PATCH 19/36] Avoid accessing namespace when possible --- .../dom/fiber/ReactDOMFiberComponent.js | 41 +++++--------- .../__tests__/ReactDOMComponent-test.js | 55 ++++++++++--------- .../shared/hooks/ReactDOMInvalidARIAHook.js | 4 +- .../hooks/ReactDOMUnknownPropertyHook.js | 4 +- .../dom/shared/utils/isCustomComponent.js | 4 +- .../dom/stack/client/ReactDOMComponent.js | 26 +++++---- .../shared/server/ReactPartialRenderer.js | 16 +++--- 7 files changed, 70 insertions(+), 80 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index b1a7bc1919f86..611c5b9076c52 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -70,10 +70,10 @@ if (__DEV__) { time: true, }; - var validatePropertiesInDevelopment = function(type, namespace, props) { - validateARIAProperties(type, props, namespace); + var validatePropertiesInDevelopment = function(type, props, domElement) { + validateARIAProperties(type, props, domElement); validateInputProperties(type, props); - validateUnknownProperties(type, props, namespace); + validateUnknownProperties(type, props, domElement); }; var warnForTextDifference = function(serverText: string, clientText: string) { @@ -307,7 +307,9 @@ var ReactDOMFiberComponent = { } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { - var isCustomComponentTag = isCustomComponent(type, props, namespaceURI); + var isCustomComponentTag = isCustomComponent(type, props, { + namespaceURI: namespaceURI, + }); // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -368,13 +370,9 @@ var ReactDOMFiberComponent = { rawProps: Object, rootContainerElement: Element | Document, ): void { - var isCustomComponentTag = isCustomComponent( - tag, - rawProps, - domElement.namespaceURI, - ); + var isCustomComponentTag = isCustomComponent(tag, rawProps, domElement); if (__DEV__) { - validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps); + validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, @@ -545,11 +543,7 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - validatePropertiesInDevelopment( - tag, - domElement.namespaceURI, - nextRawProps, - ); + validatePropertiesInDevelopment(tag, nextRawProps, domElement); } var updatePayload: null | Array = null; @@ -746,17 +740,12 @@ var ReactDOMFiberComponent = { lastRawProps: Object, nextRawProps: Object, ): void { - var namespaceURI = domElement.namespaceURI; var wasCustomComponentTag = isCustomComponent( tag, lastRawProps, - namespaceURI, - ); - var isCustomComponentTag = isCustomComponent( - tag, - nextRawProps, - namespaceURI, + domElement, ); + var isCustomComponentTag = isCustomComponent(tag, nextRawProps, domElement); // Apply the diff. updateDOMProperties( domElement, @@ -796,12 +785,8 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - var isCustomComponentTag = isCustomComponent( - tag, - rawProps, - domElement.namespaceURI, - ); - validatePropertiesInDevelopment(tag, domElement.namespaceURI, rawProps); + var isCustomComponentTag = isCustomComponent(tag, rawProps, domElement); + validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index b4f478f87f550..27f83196f1e82 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -940,35 +940,38 @@ describe('ReactDOMComponent', () => { }).toThrowError('\n\nThis DOM node was rendered by `Owner`.'); }); - it('should emit a warning once for a named custom component using shady DOM', () => { - spyOn(console, 'error'); - - var defaultCreateElement = document.createElement.bind(document); + it.only( + 'should emit a warning once for a named custom component using shady DOM', + () => { + spyOn(console, 'error'); - try { - document.createElement = element => { - var container = defaultCreateElement(element); - container.shadyRoot = {}; - return container; - }; - class ShadyComponent extends React.Component { - render() { - return ; + var defaultCreateElement = document.createElement.bind(document); + + try { + document.createElement = element => { + var container = defaultCreateElement(element); + container.shadyRoot = {}; + return container; + }; + class ShadyComponent extends React.Component { + render() { + return ; + } } + var node = document.createElement('div'); + ReactDOM.render(, node); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'ShadyComponent is using shady DOM. Using shady DOM with React can ' + + 'cause things to break subtly.', + ); + mountComponent({is: 'custom-shady-div2'}); + expectDev(console.error.calls.count()).toBe(1); + } finally { + document.createElement = defaultCreateElement; } - var node = document.createElement('div'); - ReactDOM.render(, node); - expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'ShadyComponent is using shady DOM. Using shady DOM with React can ' + - 'cause things to break subtly.', - ); - mountComponent({is: 'custom-shady-div2'}); - expectDev(console.error.calls.count()).toBe(1); - } finally { - document.createElement = defaultCreateElement; - } - }); + }, + ); it('should emit a warning once for an unnamed custom component using shady DOM', () => { spyOn(console, 'error'); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index ca0251121a551..a551a49d01162 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -145,8 +145,8 @@ function warnInvalidARIAProps(type, props, debugID) { } } -function validateProperties(type, props, namespace, debugID /* Stack only */) { - if (isCustomComponent(type, props, namespace)) { +function validateProperties(type, props, domElement, debugID /* Stack only */) { + if (isCustomComponent(type, props, domElement)) { return; } warnInvalidARIAProps(type, props, debugID); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 50931df586e22..158797c506964 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -232,8 +232,8 @@ var warnUnknownProperties = function(type, props, debugID) { } }; -function validateProperties(type, props, namespace, debugID /* Stack only */) { - if (isCustomComponent(type, props, namespace)) { +function validateProperties(type, props, domElement, debugID /* Stack only */) { + if (isCustomComponent(type, props, domElement)) { return; } warnUnknownProperties(type, props, debugID); diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index 05156574cd068..c50507bb5bc0e 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -14,11 +14,11 @@ var DOMNamespaces = require('DOMNamespaces'); var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; -function isCustomComponent(tagName, props, namespace) { +function isCustomComponent(tagName, props, domElement) { if (tagName.indexOf('-') >= 0 || typeof props.is === 'string') { // TODO: We always have a namespace with fiber. Drop the first // check when Stack is removed. - return namespace == null || namespace === HTML_NAMESPACE; + return domElement == null || domElement.namespaceURI === HTML_NAMESPACE; } return false; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 5f7a5a1e9b5da..d41ff55fbc75c 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -498,11 +498,9 @@ ReactDOMComponent.Mixin = { namespaceURI = Namespaces.html; } if (__DEV__) { - var isCustomComponentTag = isCustomComponent( - this._tag, - props, - namespaceURI, - ); + var isCustomComponentTag = isCustomComponent(this._tag, props, { + namespaceURI: namespaceURI, + }); } if (namespaceURI === Namespaces.html) { if (__DEV__) { @@ -702,7 +700,9 @@ ReactDOMComponent.Mixin = { var markup = null; if ( this._tag != null && - isCustomComponent(this._tag, props, this._namespaceURI) + isCustomComponent(this._tag, props, { + namespaceURI: this._namespaceURI, + }) ) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( @@ -886,11 +886,9 @@ ReactDOMComponent.Mixin = { } assertValidProps(this, nextProps); - var isCustomComponentTag = isCustomComponent( - this._tag, - nextProps, - this._namespaceURI, - ); + var isCustomComponentTag = isCustomComponent(this._tag, nextProps, { + namespaceURI: this._namespaceURI, + }); this._updateDOMProperties( lastProps, nextProps, @@ -964,7 +962,11 @@ ReactDOMComponent.Mixin = { } else if (registrationNameModules.hasOwnProperty(propKey)) { // Do nothing for event names. } else if (!DOMProperty.isReservedProp(propKey)) { - if (isCustomComponent(this._tag, lastProps, this._namespaceURI)) { + if ( + isCustomComponent(this._tag, lastProps, { + namespaceURI: this._namespaceURI, + }) + ) { DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey); } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 232aa0a67dd9a..be0cbf87ee65a 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -40,15 +40,15 @@ if (__DEV__) { validateProperties: validateARIAProperties, } = require('ReactDOMInvalidARIAHook'); var { - validateProperties: validateInputPropertes, + validateProperties: validateInputProperties, } = require('ReactDOMNullInputValuePropHook'); var { - validateProperties: validateUnknownPropertes, + validateProperties: validateUnknownProperties, } = require('ReactDOMUnknownPropertyHook'); - var validatePropertiesInDevelopment = function(type, namespace, props) { - validateARIAProperties(type, namespace, props); - validateInputPropertes(type, props); - validateUnknownPropertes(type, namespace, props); + var validatePropertiesInDevelopment = function(type, props, domElement) { + validateARIAProperties(type, props, domElement); + validateInputProperties(type, props); + validateUnknownProperties(type, props, domElement); }; var describeComponentFrame = require('describeComponentFrame'); @@ -278,7 +278,7 @@ function createOpenTagMarkup( propValue = createMarkupForStyles(propValue, instForDebug); } var markup = null; - if (isCustomComponent(tagLowercase, props, namespace)) { + if (isCustomComponent(tagLowercase, props, {namespaceURI: namespace})) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -775,7 +775,7 @@ class ReactDOMServerRenderer { } if (__DEV__) { - validatePropertiesInDevelopment(tag, props, namespace); + validatePropertiesInDevelopment(tag, props, {namespaceURI: namespace}); } assertValidProps(tag, props); From 76a6318101f0d6d31060a46725b2ab5257ab4f58 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 29 Aug 2017 14:42:26 -0400 Subject: [PATCH 20/36] Drop .only in ReactDOMComponent-test --- .../__tests__/ReactDOMComponent-test.js | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 27f83196f1e82..b4f478f87f550 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -940,38 +940,35 @@ describe('ReactDOMComponent', () => { }).toThrowError('\n\nThis DOM node was rendered by `Owner`.'); }); - it.only( - 'should emit a warning once for a named custom component using shady DOM', - () => { - spyOn(console, 'error'); + it('should emit a warning once for a named custom component using shady DOM', () => { + spyOn(console, 'error'); - var defaultCreateElement = document.createElement.bind(document); - - try { - document.createElement = element => { - var container = defaultCreateElement(element); - container.shadyRoot = {}; - return container; - }; - class ShadyComponent extends React.Component { - render() { - return ; - } + var defaultCreateElement = document.createElement.bind(document); + + try { + document.createElement = element => { + var container = defaultCreateElement(element); + container.shadyRoot = {}; + return container; + }; + class ShadyComponent extends React.Component { + render() { + return ; } - var node = document.createElement('div'); - ReactDOM.render(, node); - expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'ShadyComponent is using shady DOM. Using shady DOM with React can ' + - 'cause things to break subtly.', - ); - mountComponent({is: 'custom-shady-div2'}); - expectDev(console.error.calls.count()).toBe(1); - } finally { - document.createElement = defaultCreateElement; } - }, - ); + var node = document.createElement('div'); + ReactDOM.render(, node); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'ShadyComponent is using shady DOM. Using shady DOM with React can ' + + 'cause things to break subtly.', + ); + mountComponent({is: 'custom-shady-div2'}); + expectDev(console.error.calls.count()).toBe(1); + } finally { + document.createElement = defaultCreateElement; + } + }); it('should emit a warning once for an unnamed custom component using shady DOM', () => { spyOn(console, 'error'); From b29bb74b7471875ef59ef4be795971bd41840d86 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 11:52:50 -0700 Subject: [PATCH 21/36] Make isCustomComponent logic more solid --- .../dom/fiber/ReactDOMFiberComponent.js | 31 +++++++++++++++---- .../shared/hooks/ReactDOMInvalidARIAHook.js | 2 +- .../hooks/ReactDOMUnknownPropertyHook.js | 2 +- .../dom/shared/utils/isCustomComponent.js | 25 ++++++++++++--- .../dom/stack/client/ReactDOMComponent.js | 28 ++++++++--------- .../shared/server/ReactPartialRenderer.js | 2 +- 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 611c5b9076c52..e0627887384c0 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -307,9 +307,12 @@ var ReactDOMFiberComponent = { } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { - var isCustomComponentTag = isCustomComponent(type, props, { - namespaceURI: namespaceURI, - }); + var isCustomComponentTag = isCustomComponent( + type, + props, + null, + namespaceURI, + ); // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -370,7 +373,12 @@ var ReactDOMFiberComponent = { rawProps: Object, rootContainerElement: Element | Document, ): void { - var isCustomComponentTag = isCustomComponent(tag, rawProps, domElement); + var isCustomComponentTag = isCustomComponent( + tag, + rawProps, + domElement, + null, + ); if (__DEV__) { validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { @@ -744,8 +752,14 @@ var ReactDOMFiberComponent = { tag, lastRawProps, domElement, + null, + ); + var isCustomComponentTag = isCustomComponent( + tag, + nextRawProps, + domElement, + null, ); - var isCustomComponentTag = isCustomComponent(tag, nextRawProps, domElement); // Apply the diff. updateDOMProperties( domElement, @@ -785,7 +799,12 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - var isCustomComponentTag = isCustomComponent(tag, rawProps, domElement); + var isCustomComponentTag = isCustomComponent( + tag, + rawProps, + domElement, + null, + ); validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index a551a49d01162..6d764380dad01 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -146,7 +146,7 @@ function warnInvalidARIAProps(type, props, debugID) { } function validateProperties(type, props, domElement, debugID /* Stack only */) { - if (isCustomComponent(type, props, domElement)) { + if (isCustomComponent(type, props, domElement, null)) { return; } warnInvalidARIAProps(type, props, debugID); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 158797c506964..54d7a774b7306 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -233,7 +233,7 @@ var warnUnknownProperties = function(type, props, debugID) { }; function validateProperties(type, props, domElement, debugID /* Stack only */) { - if (isCustomComponent(type, props, domElement)) { + if (isCustomComponent(type, props, domElement, null)) { return; } warnUnknownProperties(type, props, debugID); diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index c50507bb5bc0e..d61c84a8d585a 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -7,18 +7,35 @@ * of patent rights can be found in the PATENTS file in the same directory. * * @providesModule isCustomComponent + * @flow */ 'use strict'; var DOMNamespaces = require('DOMNamespaces'); +var invariant = require('invariant'); var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; -function isCustomComponent(tagName, props, domElement) { +function isCustomComponent( + tagName: string, + props: Object, + domElement: Element | null, + namespaceURI: string | null, +) { + if (domElement !== null && namespaceURI !== null) { + invariant( + false, + 'Either pass domElement or namespaceURI, but not both. This error is likely ' + + 'caused by a bug in React. Please file an issue.', + ); + } if (tagName.indexOf('-') >= 0 || typeof props.is === 'string') { - // TODO: We always have a namespace with fiber. Drop the first - // check when Stack is removed. - return domElement == null || domElement.namespaceURI === HTML_NAMESPACE; + if (domElement === null) { + // TODO: We always have a namespace with fiber. Drop the first + // check when Stack is removed. + return namespaceURI === null || namespaceURI === HTML_NAMESPACE; + } + return domElement.namespaceURI === HTML_NAMESPACE; } return false; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index d41ff55fbc75c..4070b36d3f660 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -498,9 +498,12 @@ ReactDOMComponent.Mixin = { namespaceURI = Namespaces.html; } if (__DEV__) { - var isCustomComponentTag = isCustomComponent(this._tag, props, { - namespaceURI: namespaceURI, - }); + var isCustomComponentTag = isCustomComponent( + this._tag, + props, + null, + namespaceURI, + ); } if (namespaceURI === Namespaces.html) { if (__DEV__) { @@ -700,9 +703,7 @@ ReactDOMComponent.Mixin = { var markup = null; if ( this._tag != null && - isCustomComponent(this._tag, props, { - namespaceURI: this._namespaceURI, - }) + isCustomComponent(this._tag, props, null, this._namespaceURI) ) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( @@ -886,9 +887,12 @@ ReactDOMComponent.Mixin = { } assertValidProps(this, nextProps); - var isCustomComponentTag = isCustomComponent(this._tag, nextProps, { - namespaceURI: this._namespaceURI, - }); + var isCustomComponentTag = isCustomComponent( + this._tag, + nextProps, + null, + this._namespaceURI, + ); this._updateDOMProperties( lastProps, nextProps, @@ -962,11 +966,7 @@ ReactDOMComponent.Mixin = { } else if (registrationNameModules.hasOwnProperty(propKey)) { // Do nothing for event names. } else if (!DOMProperty.isReservedProp(propKey)) { - if ( - isCustomComponent(this._tag, lastProps, { - namespaceURI: this._namespaceURI, - }) - ) { + if (isCustomComponent(this._tag, lastProps, null, this._namespaceURI)) { DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey); } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index be0cbf87ee65a..26aa92980594c 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -278,7 +278,7 @@ function createOpenTagMarkup( propValue = createMarkupForStyles(propValue, instForDebug); } var markup = null; - if (isCustomComponent(tagLowercase, props, {namespaceURI: namespace})) { + if (isCustomComponent(tagLowercase, props, null, namespace)) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, From 0a2aec473ec83de5e4ead023ecc5ad56d3e26c05 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 12:04:22 -0700 Subject: [PATCH 22/36] Do attribute name check earlier --- src/renderers/dom/shared/DOMProperty.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index fbf4f05e27877..83bdb28ef052f 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -208,15 +208,15 @@ var DOMProperty = { if (DOMProperty.isReservedProp(name)) { return false; } - if (value === null) { - return true; - } if ( (name[0] === 'o' || name[0] === 'O') && (name[1] === 'n' || name[1] === 'N') ) { return false; } + if (value === null) { + return true; + } switch (typeof value) { case 'boolean': return DOMProperty.shouldAttributeAcceptBooleanValue(name); From 501e86d368550f45eab4cd788959faf3ff2df55c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 12:36:22 -0700 Subject: [PATCH 23/36] Fix fbjs import --- src/renderers/dom/shared/utils/isCustomComponent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index d61c84a8d585a..d9f17e44e421d 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -13,7 +13,7 @@ 'use strict'; var DOMNamespaces = require('DOMNamespaces'); -var invariant = require('invariant'); +var invariant = require('fbjs/lib/invariant'); var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; function isCustomComponent( From 8d1f487392093033514ab787b4a708c7386756d4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 15:25:54 -0700 Subject: [PATCH 24/36] Revert unintentional edit --- fixtures/packaging/babel-standalone/dev.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fixtures/packaging/babel-standalone/dev.html b/fixtures/packaging/babel-standalone/dev.html index 1bf91ddc422eb..9c3a931badcc4 100644 --- a/fixtures/packaging/babel-standalone/dev.html +++ b/fixtures/packaging/babel-standalone/dev.html @@ -1,12 +1,12 @@ - - + +
From 72666fa1cfc2fad722534909c247f38f2ea7ed25 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 15:38:32 -0700 Subject: [PATCH 25/36] Re-allow "data" attribute We intentionally allowed it. --- src/renderers/dom/shared/DOMProperty.js | 1 - .../dom/shared/__tests__/ReactDOMComponent-test.js | 8 ++------ .../shared/__tests__/ReactDOMServerIntegration-test.js | 7 ++++--- .../dom/shared/hooks/ReactDOMUnknownPropertyHook.js | 10 ---------- 4 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 83bdb28ef052f..e96e4bc3cb365 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -18,7 +18,6 @@ var invariant = require('fbjs/lib/invariant'); var RESERVED_PROPS = { aria: true, children: true, - data: true, dangerouslysetinnerhtml: true, autofocus: true, defaultvalue: true, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index b4f478f87f550..34f0dc5ec07d7 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -829,13 +829,9 @@ describe('ReactDOMComponent', () => { it('should warn on props reserved for future use', () => { spyOn(console, 'error'); - ReactTestUtils.renderIntoDocument(
); - expectDev(console.error.calls.count()).toBe(2); + ReactTestUtils.renderIntoDocument(
); + expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'The `data` attribute is reserved for future use in React, and will be ignored. ' + - 'Pass individual `data-` attributes instead.', - ); - expectDev(console.error.calls.argsFor(1)[0]).toContain( 'The `aria` attribute is reserved for future use in React, and will be ignored. ' + 'Pass individual `aria-` attributes instead.', ); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index b81834404f1a4..5338476307a65 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -893,9 +893,10 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('data-foo')).toBe('bar'); }); - itRenders('no "data" attribute', async render => { - const e = await render(
, 1); - expect(e.hasAttribute('data')).toBe(false); + itRenders('"data" attribute', async render => { + // For `` acts as `src`. + const e = await render(); + expect(e.getAttribute('data')).toBe(true); }); itRenders('no unknown data- attributes with null value', async render => { diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 54d7a774b7306..c6fe28bd4a82b 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -115,16 +115,6 @@ if (__DEV__) { return true; } - if (lowerCasedName === 'data') { - warning( - false, - 'The `data` attribute is reserved for future use in React, and will be ignored. ' + - 'Pass individual `data-` attributes instead.', - ); - warnedProperties[name] = true; - return true; - } - if ( lowerCasedName === 'is' && value !== null && From cb687ed95858e1a588dcc3b301100744838a2cb7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 16:01:00 -0700 Subject: [PATCH 26/36] Use stricter check when attaching events --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index e0627887384c0..028b739161185 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -233,7 +233,7 @@ function setInitialDOMProperties( } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) { // Noop } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } @@ -715,7 +715,7 @@ var ReactDOMFiberComponent = { } else if (propKey === SUPPRESS_CONTENT_EDITABLE_WARNING) { // Noop } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { // We eagerly listen to this even though we haven't committed yet. if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); @@ -974,7 +974,7 @@ var ReactDOMFiberComponent = { } } } else if (registrationNameModules.hasOwnProperty(propKey)) { - if (nextProp) { + if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } From 1d6137956fddf4e70244f8f7d4ceaf73dfa88986 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 16:31:44 -0700 Subject: [PATCH 27/36] Pass SVG boolean attributes with correct casing --- src/renderers/dom/shared/SVGDOMPropertyConfig.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/SVGDOMPropertyConfig.js b/src/renderers/dom/shared/SVGDOMPropertyConfig.js index c258ba0cf280f..0254d8ee0c20c 100644 --- a/src/renderers/dom/shared/SVGDOMPropertyConfig.js +++ b/src/renderers/dom/shared/SVGDOMPropertyConfig.js @@ -125,6 +125,11 @@ var SVGDOMPropertyConfig = { externalResourcesRequired: HAS_STRING_BOOLEAN_VALUE, preserveAlpha: HAS_STRING_BOOLEAN_VALUE, }, + DOMAttributeNames: { + autoReverse: 'autoReverse', + externalResourcesRequired: 'externalResourcesRequired', + preserveAlpha: 'preserveAlpha', + }, DOMAttributeNamespaces: { xlinkActuate: NS.xlink, xlinkArcrole: NS.xlink, @@ -137,7 +142,6 @@ var SVGDOMPropertyConfig = { xmlLang: NS.xml, xmlSpace: NS.xml, }, - DOMAttributeNames: {}, }; var CAMELIZE = /[\-\:]([a-z])/g; From 2b0f61a229fa2ff755d9e4daa701aa1181a71d41 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 29 Aug 2017 16:32:08 -0700 Subject: [PATCH 28/36] Fix the test --- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 5338476307a65..4ea6a4c285689 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -896,7 +896,7 @@ describe('ReactDOMServerIntegration', () => { itRenders('"data" attribute', async render => { // For `` acts as `src`. const e = await render(); - expect(e.getAttribute('data')).toBe(true); + expect(e.getAttribute('data')).toBe('hello'); }); itRenders('no unknown data- attributes with null value', async render => { From 1590c2a856aa604fa28c29bac273782d6ac3fc2f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 02:30:49 -0700 Subject: [PATCH 29/36] Undo the SVG dashed-name fix Per conversation with @sebmarkbage we decided that the fix is too complicated, and it's unfortunate it depends on the DOM element. It's only relevant for super rare tags that aren't even working consistently across browsers so we'll leave it unfixed for now. --- .../dom/fiber/ReactDOMFiberComponent.js | 14 ++------- .../__tests__/ReactDOMComponent-test.js | 5 +++- .../ReactDOMServerIntegration-test.js | 20 ++++++++----- .../shared/hooks/ReactDOMInvalidARIAHook.js | 8 ++--- .../hooks/ReactDOMUnknownPropertyHook.js | 8 ++--- .../dom/shared/utils/isCustomComponent.js | 29 ++----------------- .../dom/stack/client/ReactDOMComponent.js | 8 ++--- .../shared/server/ReactPartialRenderer.js | 10 +++---- 8 files changed, 35 insertions(+), 67 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 028b739161185..04674f7f34e40 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -71,9 +71,9 @@ if (__DEV__) { }; var validatePropertiesInDevelopment = function(type, props, domElement) { - validateARIAProperties(type, props, domElement); + validateARIAProperties(type, props); validateInputProperties(type, props); - validateUnknownProperties(type, props, domElement); + validateUnknownProperties(type, props); }; var warnForTextDifference = function(serverText: string, clientText: string) { @@ -310,8 +310,6 @@ var ReactDOMFiberComponent = { var isCustomComponentTag = isCustomComponent( type, props, - null, - namespaceURI, ); // Should this check be gated by parent namespace? Not sure we want to // allow or . @@ -376,8 +374,6 @@ var ReactDOMFiberComponent = { var isCustomComponentTag = isCustomComponent( tag, rawProps, - domElement, - null, ); if (__DEV__) { validatePropertiesInDevelopment(tag, rawProps, domElement); @@ -751,14 +747,10 @@ var ReactDOMFiberComponent = { var wasCustomComponentTag = isCustomComponent( tag, lastRawProps, - domElement, - null, ); var isCustomComponentTag = isCustomComponent( tag, nextRawProps, - domElement, - null, ); // Apply the diff. updateDOMProperties( @@ -802,8 +794,6 @@ var ReactDOMFiberComponent = { var isCustomComponentTag = isCustomComponent( tag, rawProps, - domElement, - null, ); validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 34f0dc5ec07d7..09b64ef39cb05 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -2212,7 +2212,10 @@ describe('ReactDOMComponent', () => { }); if (ReactDOMFeatureFlags.useFiber) { - describe('Hyphenated SVG elements', function() { + // This is currently broken (and has been broken for a while). + // We had a fix based on reading namespace, but it was too convoluted. + // TODO: a proper fix that would happen at the diffing stage. + describe.skip('Hyphenated SVG elements', function() { it('the font-face element is not a custom element', function() { spyOn(console, 'error'); var el = ReactTestUtils.renderIntoDocument( diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 4ea6a4c285689..b04832949026f 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -961,14 +961,18 @@ describe('ReactDOMServerIntegration', () => { }, ); - itRenders( - 'known SVG attributes for elements with dashes in tag', - async render => { - const e = await render(); - expect(e.firstChild.hasAttribute('accentHeight')).toBe(false); - expect(e.firstChild.getAttribute('accent-height')).toBe('10'); - }, - ); + // This is currently broken (and has been broken for a while). + // We had a fix based on reading namespace, but it was too convoluted. + // TODO: a proper fix that would happen at the diffing stage. + // + // itRenders( + // 'known SVG attributes for elements with dashes in tag', + // async render => { + // const e = await render(); + // expect(e.firstChild.hasAttribute('accentHeight')).toBe(false); + // expect(e.firstChild.getAttribute('accent-height')).toBe('10'); + // }, + // ); itRenders('cased custom attributes', async render => { const e = await render(
); diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index 6d764380dad01..705c5aa207d20 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -145,8 +145,8 @@ function warnInvalidARIAProps(type, props, debugID) { } } -function validateProperties(type, props, domElement, debugID /* Stack only */) { - if (isCustomComponent(type, props, domElement, null)) { +function validateProperties(type, props, debugID /* Stack only */) { + if (isCustomComponent(type, props)) { return; } warnInvalidARIAProps(type, props, debugID); @@ -158,12 +158,12 @@ var ReactDOMInvalidARIAHook = { // Stack onBeforeMountComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, null, debugID); + validateProperties(element.type, element.props, debugID); } }, onBeforeUpdateComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, null, debugID); + validateProperties(element.type, element.props, debugID); } }, }; diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index c6fe28bd4a82b..ac7e785359967 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -222,8 +222,8 @@ var warnUnknownProperties = function(type, props, debugID) { } }; -function validateProperties(type, props, domElement, debugID /* Stack only */) { - if (isCustomComponent(type, props, domElement, null)) { +function validateProperties(type, props, debugID /* Stack only */) { + if (isCustomComponent(type, props)) { return; } warnUnknownProperties(type, props, debugID); @@ -235,12 +235,12 @@ var ReactDOMUnknownPropertyHook = { // Stack onBeforeMountComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, null, debugID); + validateProperties(element.type, element.props, debugID); } }, onBeforeUpdateComponent(debugID, element) { if (__DEV__ && element != null && typeof element.type === 'string') { - validateProperties(element.type, element.props, null, debugID); + validateProperties(element.type, element.props, debugID); } }, }; diff --git a/src/renderers/dom/shared/utils/isCustomComponent.js b/src/renderers/dom/shared/utils/isCustomComponent.js index d9f17e44e421d..0d730a877e8f8 100644 --- a/src/renderers/dom/shared/utils/isCustomComponent.js +++ b/src/renderers/dom/shared/utils/isCustomComponent.js @@ -12,33 +12,8 @@ 'use strict'; -var DOMNamespaces = require('DOMNamespaces'); -var invariant = require('fbjs/lib/invariant'); -var HTML_NAMESPACE = DOMNamespaces.Namespaces.html; - -function isCustomComponent( - tagName: string, - props: Object, - domElement: Element | null, - namespaceURI: string | null, -) { - if (domElement !== null && namespaceURI !== null) { - invariant( - false, - 'Either pass domElement or namespaceURI, but not both. This error is likely ' + - 'caused by a bug in React. Please file an issue.', - ); - } - if (tagName.indexOf('-') >= 0 || typeof props.is === 'string') { - if (domElement === null) { - // TODO: We always have a namespace with fiber. Drop the first - // check when Stack is removed. - return namespaceURI === null || namespaceURI === HTML_NAMESPACE; - } - return domElement.namespaceURI === HTML_NAMESPACE; - } - - return false; +function isCustomComponent(tagName: string, props: Object) { + return tagName.indexOf('-') >= 0 || typeof props.is === 'string'; } module.exports = isCustomComponent; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 4070b36d3f660..43c0e3e576aa5 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -501,8 +501,6 @@ ReactDOMComponent.Mixin = { var isCustomComponentTag = isCustomComponent( this._tag, props, - null, - namespaceURI, ); } if (namespaceURI === Namespaces.html) { @@ -703,7 +701,7 @@ ReactDOMComponent.Mixin = { var markup = null; if ( this._tag != null && - isCustomComponent(this._tag, props, null, this._namespaceURI) + isCustomComponent(this._tag, props) ) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( @@ -890,8 +888,6 @@ ReactDOMComponent.Mixin = { var isCustomComponentTag = isCustomComponent( this._tag, nextProps, - null, - this._namespaceURI, ); this._updateDOMProperties( lastProps, @@ -966,7 +962,7 @@ ReactDOMComponent.Mixin = { } else if (registrationNameModules.hasOwnProperty(propKey)) { // Do nothing for event names. } else if (!DOMProperty.isReservedProp(propKey)) { - if (isCustomComponent(this._tag, lastProps, null, this._namespaceURI)) { + if (isCustomComponent(this._tag, lastProps)) { DOMPropertyOperations.deleteValueForAttribute(getNode(this), propKey); } else { DOMPropertyOperations.deleteValueForProperty(getNode(this), propKey); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 26aa92980594c..f86e45b6b5351 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -45,10 +45,10 @@ if (__DEV__) { var { validateProperties: validateUnknownProperties, } = require('ReactDOMUnknownPropertyHook'); - var validatePropertiesInDevelopment = function(type, props, domElement) { - validateARIAProperties(type, props, domElement); + var validatePropertiesInDevelopment = function(type, props) { + validateARIAProperties(type, props); validateInputProperties(type, props); - validateUnknownProperties(type, props, domElement); + validateUnknownProperties(type, props); }; var describeComponentFrame = require('describeComponentFrame'); @@ -278,7 +278,7 @@ function createOpenTagMarkup( propValue = createMarkupForStyles(propValue, instForDebug); } var markup = null; - if (isCustomComponent(tagLowercase, props, null, namespace)) { + if (isCustomComponent(tagLowercase, props)) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -775,7 +775,7 @@ class ReactDOMServerRenderer { } if (__DEV__) { - validatePropertiesInDevelopment(tag, props, {namespaceURI: namespace}); + validatePropertiesInDevelopment(tag, props); } assertValidProps(tag, props); From cb883a6553e9e3a8ca3ad2bc7e5d8c44021e40e2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 11:13:06 -0700 Subject: [PATCH 30/36] Prettier --- .../dom/fiber/ReactDOMFiberComponent.js | 25 ++++--------------- .../dom/stack/client/ReactDOMComponent.js | 15 +++-------- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 04674f7f34e40..b1e11bd37befd 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -307,10 +307,7 @@ var ReactDOMFiberComponent = { } if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { - var isCustomComponentTag = isCustomComponent( - type, - props, - ); + var isCustomComponentTag = isCustomComponent(type, props); // Should this check be gated by parent namespace? Not sure we want to // allow or . warning( @@ -371,10 +368,7 @@ var ReactDOMFiberComponent = { rawProps: Object, rootContainerElement: Element | Document, ): void { - var isCustomComponentTag = isCustomComponent( - tag, - rawProps, - ); + var isCustomComponentTag = isCustomComponent(tag, rawProps); if (__DEV__) { validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { @@ -744,14 +738,8 @@ var ReactDOMFiberComponent = { lastRawProps: Object, nextRawProps: Object, ): void { - var wasCustomComponentTag = isCustomComponent( - tag, - lastRawProps, - ); - var isCustomComponentTag = isCustomComponent( - tag, - nextRawProps, - ); + var wasCustomComponentTag = isCustomComponent(tag, lastRawProps); + var isCustomComponentTag = isCustomComponent(tag, nextRawProps); // Apply the diff. updateDOMProperties( domElement, @@ -791,10 +779,7 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - var isCustomComponentTag = isCustomComponent( - tag, - rawProps, - ); + var isCustomComponentTag = isCustomComponent(tag, rawProps); validatePropertiesInDevelopment(tag, rawProps, domElement); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 43c0e3e576aa5..daf427fcc9e7c 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -498,10 +498,7 @@ ReactDOMComponent.Mixin = { namespaceURI = Namespaces.html; } if (__DEV__) { - var isCustomComponentTag = isCustomComponent( - this._tag, - props, - ); + var isCustomComponentTag = isCustomComponent(this._tag, props); } if (namespaceURI === Namespaces.html) { if (__DEV__) { @@ -699,10 +696,7 @@ ReactDOMComponent.Mixin = { propValue = createMarkupForStyles(propValue, this); } var markup = null; - if ( - this._tag != null && - isCustomComponent(this._tag, props) - ) { + if (this._tag != null && isCustomComponent(this._tag, props)) { if (!DOMProperty.isReservedProp(propKey)) { markup = DOMMarkupOperations.createMarkupForCustomAttribute( propKey, @@ -885,10 +879,7 @@ ReactDOMComponent.Mixin = { } assertValidProps(this, nextProps); - var isCustomComponentTag = isCustomComponent( - this._tag, - nextProps, - ); + var isCustomComponentTag = isCustomComponent(this._tag, nextProps); this._updateDOMProperties( lastProps, nextProps, From 32f6321c07136c2b12f073b2d0ba1c67b60c26b7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 11:54:33 -0700 Subject: [PATCH 31/36] Fix lint --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index b1e11bd37befd..3cb10d7fabd95 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -70,7 +70,7 @@ if (__DEV__) { time: true, }; - var validatePropertiesInDevelopment = function(type, props, domElement) { + var validatePropertiesInDevelopment = function(type, props) { validateARIAProperties(type, props); validateInputProperties(type, props); validateUnknownProperties(type, props); From 6d9c0e0f04dabd7afd4e4488b0f1cd7d1493ce1f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 12:32:19 -0700 Subject: [PATCH 32/36] Fix flow --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 3cb10d7fabd95..4fe120cdaf479 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -370,7 +370,7 @@ var ReactDOMFiberComponent = { ): void { var isCustomComponentTag = isCustomComponent(tag, rawProps); if (__DEV__) { - validatePropertiesInDevelopment(tag, rawProps, domElement); + validatePropertiesInDevelopment(tag, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, @@ -541,7 +541,7 @@ var ReactDOMFiberComponent = { rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { - validatePropertiesInDevelopment(tag, nextRawProps, domElement); + validatePropertiesInDevelopment(tag, nextRawProps); } var updatePayload: null | Array = null; @@ -780,7 +780,7 @@ var ReactDOMFiberComponent = { ): null | Array { if (__DEV__) { var isCustomComponentTag = isCustomComponent(tag, rawProps); - validatePropertiesInDevelopment(tag, rawProps, domElement); + validatePropertiesInDevelopment(tag, rawProps); if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( false, From 10e0c09c852c5be69fd88c55947ef656565577c5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 14:57:36 -0700 Subject: [PATCH 33/36] Pass "aria" through but still warn --- src/renderers/dom/shared/DOMProperty.js | 1 - src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js | 2 +- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 5 +++-- .../dom/shared/hooks/ReactDOMUnknownPropertyHook.js | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index e96e4bc3cb365..3c6e4309e185e 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -16,7 +16,6 @@ var invariant = require('fbjs/lib/invariant'); // These attributes should be all lowercase to allow for // case insensitive checks var RESERVED_PROPS = { - aria: true, children: true, dangerouslysetinnerhtml: true, autofocus: true, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 09b64ef39cb05..b48600fbe1942 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -832,7 +832,7 @@ describe('ReactDOMComponent', () => { ReactTestUtils.renderIntoDocument(
); expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'The `aria` attribute is reserved for future use in React, and will be ignored. ' + + 'The `aria` attribute is reserved for future use in React. ' + 'Pass individual `aria-` attributes instead.', ); }); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index b04832949026f..c6fc087b9aef7 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -846,9 +846,10 @@ describe('ReactDOMServerIntegration', () => { expect(e.hasAttribute('aria-label')).toBe(false); }); - itRenders('no "aria" attribute', async render => { + itRenders('"aria" attribute with a warning', async render => { + // Reserved for future use. const e = await render(
, 1); - expect(e.hasAttribute('aria')).toBe(false); + expect(e.getAttribute('aria')).toBe('hello'); }); }); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index ac7e785359967..ac43ba6d80850 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -108,7 +108,7 @@ if (__DEV__) { if (lowerCasedName === 'aria') { warning( false, - 'The `aria` attribute is reserved for future use in React, and will be ignored. ' + + 'The `aria` attribute is reserved for future use in React. ' + 'Pass individual `aria-` attributes instead.', ); warnedProperties[name] = true; From ba71ec13251f66717030d00c473663b9f564b05d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 14:58:38 -0700 Subject: [PATCH 34/36] Remove special cases for onfocusin, onfocusout They're covered by event handler code now. --- src/renderers/dom/shared/DOMProperty.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 3c6e4309e185e..b3b56a656395f 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -23,8 +23,6 @@ var RESERVED_PROPS = { defaultchecked: true, innerhtml: true, suppresscontenteditablewarning: true, - onfocusin: true, - onfocusout: true, style: true, }; From dc760afc4bbe030cf2e50c9c6f26ad95709fe28b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 17:34:33 -0700 Subject: [PATCH 35/36] Add a more specific warning for unknown events --- .../__tests__/ReactDOMComponent-test.js | 48 +++++++++++++++++++ .../hooks/ReactDOMUnknownPropertyHook.js | 11 +++++ 2 files changed, 59 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index b48600fbe1942..4c595735b80f3 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -178,6 +178,54 @@ describe('ReactDOMComponent', () => { ); }); + it('should warn for unknown string event handlers', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); + expect(container.firstChild.onUnknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onunknown')).toBe(false); + expect(container.firstChild.onunknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); + expect(container.firstChild['on-unknown']).toBe(undefined); + expectDev(console.error.calls.count(0)).toBe(3); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', + ); + }); + + it('should warn for unknown function event handlers', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); + expect(container.firstChild.onUnknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('onunknown')).toBe(false); + expect(container.firstChild.onunknown).toBe(undefined); + ReactDOM.render(
, container); + expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); + expect(container.firstChild['on-unknown']).toBe(undefined); + expectDev(console.error.calls.count(0)).toBe(3); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( + 'Warning: Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', + ); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( + 'Warning: Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', + ); + }); + it('should not warn for "0" as a unitless style value', () => { spyOn(console, 'error'); diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index ac43ba6d80850..1ce2a90fecb63 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -79,6 +79,17 @@ if (__DEV__) { return true; } + if (lowerCasedName.indexOf('on') === 0) { + warning( + false, + 'Unknown event handler property `%s`. It will be ignored.%s', + name, + getStackAddendum(debugID), + ); + warnedProperties[name] = true; + return true; + } + // Let the ARIA attribute hook validate ARIA attributes if (ARIA_NAME_REGEX.test(name)) { return true; From 10950c46509877277f50f10eb7e4e6a51f47b2c5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 30 Aug 2017 18:00:43 -0700 Subject: [PATCH 36/36] Pass badly cased React attributes through with warning --- src/renderers/dom/shared/DOMProperty.js | 14 +++++++------- .../dom/shared/__tests__/ReactDOMComponent-test.js | 11 +++++++++++ .../__tests__/ReactDOMServerIntegration-test.js | 5 +++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index b3b56a656395f..be3c8a1233b40 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -17,12 +17,12 @@ var invariant = require('fbjs/lib/invariant'); // case insensitive checks var RESERVED_PROPS = { children: true, - dangerouslysetinnerhtml: true, - autofocus: true, - defaultvalue: true, - defaultchecked: true, - innerhtml: true, - suppresscontenteditablewarning: true, + dangerouslySetInnerHTML: true, + autoFocus: true, + defaultValue: true, + defaultChecked: true, + innerHTML: true, + suppressContentEditableWarning: true, style: true, }; @@ -259,7 +259,7 @@ var DOMProperty = { * @return {boolean} If the name is within reserved props */ isReservedProp(name) { - return RESERVED_PROPS.hasOwnProperty(name.toLowerCase()); + return RESERVED_PROPS.hasOwnProperty(name); }, injection: DOMPropertyInjection, diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 4c595735b80f3..b37e4e647971a 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -226,6 +226,17 @@ describe('ReactDOMComponent', () => { ); }); + it('should warn for badly cased React attributes', () => { + spyOn(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render(
, container); + expect(container.firstChild.getAttribute('CHILDREN')).toBe('5'); + expectDev(console.error.calls.count(0)).toBe(1); + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( + 'Warning: Invalid DOM property `CHILDREN`. Did you mean `children`?\n in div (at **)', + ); + }); + it('should not warn for "0" as a unitless style value', () => { spyOn(console, 'error'); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index c6fc087b9aef7..0d4256117053b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -894,6 +894,11 @@ describe('ReactDOMServerIntegration', () => { expect(e.getAttribute('data-foo')).toBe('bar'); }); + itRenders('badly cased reserved attributes', async render => { + const e = await render(
, 1); + expect(e.getAttribute('CHILDREN')).toBe('5'); + }); + itRenders('"data" attribute', async render => { // For `` acts as `src`. const e = await render();