@samitouri / QOS-React / commits / d9869a4561

Revert "Refactor DOM attribute code (#11804)" (#11814)

This reverts commit 47783e878d62ed96ea27290d1cdbb90b83c417c4.

Dan Abramov committed Dec 8, 2017 at 21:05 UTC d9869a45616884512220e162d1d3589f48a26fb0
7 files changed +194 -305
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
-110
@@ -442,97 +442,6 @@ describe('ReactDOMComponent', () => {
442 expect(container.firstChild.className).toEqual('');
443 });
444
445 - it('should not set null/undefined attributes', () => {
446 - var container = document.createElement('div');
447 - // Initial render.
448 - ReactDOM.render(<img src={null} data-foo={undefined} />, container);
449 - var node = container.firstChild;
450 - expect(node.hasAttribute('src')).toBe(false);
451 - expect(node.hasAttribute('data-foo')).toBe(false);
452 - // Update in one direction.
453 - ReactDOM.render(<img src={undefined} data-foo={null} />, container);
454 - expect(node.hasAttribute('src')).toBe(false);
455 - expect(node.hasAttribute('data-foo')).toBe(false);
456 - // Update in another direction.
457 - ReactDOM.render(<img src={null} data-foo={undefined} />, container);
458 - expect(node.hasAttribute('src')).toBe(false);
459 - expect(node.hasAttribute('data-foo')).toBe(false);
460 - // Removal.
461 - ReactDOM.render(<img />, container);
462 - expect(node.hasAttribute('src')).toBe(false);
463 - expect(node.hasAttribute('data-foo')).toBe(false);
464 - // Addition.
465 - ReactDOM.render(<img src={undefined} data-foo={null} />, container);
466 - expect(node.hasAttribute('src')).toBe(false);
467 - expect(node.hasAttribute('data-foo')).toBe(false);
468 - });
469 -
470 - it('should apply React-specific aliases to HTML elements', () => {
471 - var container = document.createElement('div');
472 - ReactDOM.render(<form acceptCharset="foo" />, container);
473 - var node = container.firstChild;
474 - // Test attribute initialization.
475 - expect(node.getAttribute('accept-charset')).toBe('foo');
476 - expect(node.hasAttribute('acceptCharset')).toBe(false);
477 - // Test attribute update.
478 - ReactDOM.render(<form acceptCharset="boo" />, container);
479 - expect(node.getAttribute('accept-charset')).toBe('boo');
480 - expect(node.hasAttribute('acceptCharset')).toBe(false);
481 - // Test attribute removal by setting to null.
482 - ReactDOM.render(<form acceptCharset={null} />, container);
483 - expect(node.hasAttribute('accept-charset')).toBe(false);
484 - expect(node.hasAttribute('acceptCharset')).toBe(false);
485 - // Restore.
486 - ReactDOM.render(<form acceptCharset="foo" />, container);
487 - expect(node.getAttribute('accept-charset')).toBe('foo');
488 - expect(node.hasAttribute('acceptCharset')).toBe(false);
489 - // Test attribute removal by setting to undefined.
490 - ReactDOM.render(<form acceptCharset={undefined} />, container);
491 - expect(node.hasAttribute('accept-charset')).toBe(false);
492 - expect(node.hasAttribute('acceptCharset')).toBe(false);
493 - // Restore.
494 - ReactDOM.render(<form acceptCharset="foo" />, container);
495 - expect(node.getAttribute('accept-charset')).toBe('foo');
496 - expect(node.hasAttribute('acceptCharset')).toBe(false);
497 - // Test attribute removal.
498 - ReactDOM.render(<form />, container);
499 - expect(node.hasAttribute('accept-charset')).toBe(false);
500 - expect(node.hasAttribute('acceptCharset')).toBe(false);
501 - });
502 -
503 - it('should apply React-specific aliases to SVG elements', () => {
504 - var container = document.createElement('div');
505 - ReactDOM.render(<svg arabicForm="foo" />, container);
506 - var node = container.firstChild;
507 - // Test attribute initialization.
508 - expect(node.getAttribute('arabic-form')).toBe('foo');
509 - expect(node.hasAttribute('arabicForm')).toBe(false);
510 - // Test attribute update.
511 - ReactDOM.render(<svg arabicForm="boo" />, container);
512 - expect(node.getAttribute('arabic-form')).toBe('boo');
513 - expect(node.hasAttribute('arabicForm')).toBe(false);
514 - // Test attribute removal by setting to null.
515 - ReactDOM.render(<svg arabicForm={null} />, container);
516 - expect(node.hasAttribute('arabic-form')).toBe(false);
517 - expect(node.hasAttribute('arabicForm')).toBe(false);
518 - // Restore.
519 - ReactDOM.render(<svg arabicForm="foo" />, container);
520 - expect(node.getAttribute('arabic-form')).toBe('foo');
521 - expect(node.hasAttribute('arabicForm')).toBe(false);
522 - // Test attribute removal by setting to undefined.
523 - ReactDOM.render(<svg arabicForm={undefined} />, container);
524 - expect(node.hasAttribute('arabic-form')).toBe(false);
525 - expect(node.hasAttribute('arabicForm')).toBe(false);
526 - // Restore.
527 - ReactDOM.render(<svg arabicForm="foo" />, container);
528 - expect(node.getAttribute('arabic-form')).toBe('foo');
529 - expect(node.hasAttribute('arabicForm')).toBe(false);
530 - // Test attribute removal.
531 - ReactDOM.render(<svg />, container);
532 - expect(node.hasAttribute('arabic-form')).toBe(false);
533 - expect(node.hasAttribute('arabicForm')).toBe(false);
534 - });
535 -
445 it('should properly update custom attributes on custom elements', () => {
446 const container = document.createElement('div');
447 ReactDOM.render(<some-custom-element foo="bar" />, container);
@@ -542,25 +451,6 @@ describe('ReactDOMComponent', () => {
451 expect(node.getAttribute('bar')).toBe('buzz');
452 });
453
545 - it('should not apply React-specific aliases to custom elements', () => {
546 - var container = document.createElement('div');
547 - ReactDOM.render(<some-custom-element arabicForm="foo" />, container);
548 - var node = container.firstChild;
549 - // Should not get transformed to arabic-form as SVG would be.
550 - expect(node.getAttribute('arabicForm')).toBe('foo');
551 - expect(node.hasAttribute('arabic-form')).toBe(false);
552 - // Test attribute update.
553 - ReactDOM.render(<some-custom-element arabicForm="boo" />, container);
554 - expect(node.getAttribute('arabicForm')).toBe('boo');
555 - // Test attribute removal and addition.
556 - ReactDOM.render(<some-custom-element acceptCharset="buzz" />, container);
557 - // Verify the previous attribute was removed.
558 - expect(node.hasAttribute('arabicForm')).toBe(false);
559 - // Should not get transformed to accept-charset as HTML would be.
560 - expect(node.getAttribute('acceptCharset')).toBe('buzz');
561 - expect(node.hasAttribute('accept-charset')).toBe(false);
562 - });
563 -
454 it('should clear a single style prop when changing `style`', () => {
455 let styles = {display: 'none', color: 'red'};
456 const container = document.createElement('div');
packages/react-dom/src/client/DOMPropertyOperations.js
+105 -71
@@ -3,33 +3,51 @@
3 *
4 * This source code is licensed under the MIT license found in the
5 * LICENSE file in the root directory of this source tree.
6 - *
7 - * @flow
6 */
7
8 import {
9 + ID_ATTRIBUTE_NAME,
10 + ROOT_ATTRIBUTE_NAME,
11 getPropertyInfo,
12 - shouldSkipAttribute,
13 - shouldTreatAttributeValueAsNull,
12 + shouldSetAttribute,
13 isAttributeNameSafe,
14 } from '../shared/DOMProperty';
15
16 +// shouldIgnoreValue() is currently duplicated in DOMMarkupOperations.
17 +// TODO: Find a better place for this.
18 +function shouldIgnoreValue(propertyInfo, value) {
19 + return (
20 + value == null ||
21 + (propertyInfo.hasBooleanValue && !value) ||
22 + (propertyInfo.hasNumericValue && isNaN(value)) ||
23 + (propertyInfo.hasPositiveNumericValue && value < 1) ||
24 + (propertyInfo.hasOverloadedBooleanValue && value === false)
25 + );
26 +}
27 +
28 +/**
29 + * Operations for dealing with DOM properties.
30 + */
31 +
32 +export function setAttributeForID(node, id) {
33 + node.setAttribute(ID_ATTRIBUTE_NAME, id);
34 +}
35 +
36 +export function setAttributeForRoot(node) {
37 + node.setAttribute(ROOT_ATTRIBUTE_NAME, '');
38 +}
39 +
40 /**
41 * Get the value for a property on a node. Only used in DEV for SSR validation.
42 * The "expected" argument is used as a hint of what the expected value is.
43 * Some properties have multiple equivalent values.
44 */
22 -export function getValueForProperty(
23 - node: Element,
24 - name: string,
25 - expected: mixed,
26 -): mixed {
45 +export function getValueForProperty(node, name, expected) {
46 if (__DEV__) {
47 const propertyInfo = getPropertyInfo(name);
48 if (propertyInfo) {
49 if (propertyInfo.mustUseProperty) {
31 - const {propertyName} = propertyInfo;
32 - return (node: any)[propertyName];
50 + return node[propertyInfo.propertyName];
51 } else {
52 const attributeName = propertyInfo.attributeName;
53
@@ -41,16 +59,16 @@ export function getValueForProperty(
59 if (value === '') {
60 return true;
61 }
44 - if (shouldTreatAttributeValueAsNull(name, expected, false)) {
62 + if (shouldIgnoreValue(propertyInfo, expected)) {
63 return value;
64 }
47 - if (value === '' + (expected: any)) {
65 + if (value === '' + expected) {
66 return expected;
67 }
68 return value;
69 }
70 } else if (node.hasAttribute(attributeName)) {
53 - if (shouldTreatAttributeValueAsNull(name, expected, false)) {
71 + if (shouldIgnoreValue(propertyInfo, expected)) {
72 // We had an attribute but shouldn't have had one, so read it
73 // for the error message.
74 return node.getAttribute(attributeName);
@@ -67,9 +85,9 @@ export function getValueForProperty(
85 stringValue = node.getAttribute(attributeName);
86 }
87
70 - if (shouldTreatAttributeValueAsNull(name, expected, false)) {
88 + if (shouldIgnoreValue(propertyInfo, expected)) {
89 return stringValue === null ? expected : stringValue;
72 - } else if (stringValue === '' + (expected: any)) {
90 + } else if (stringValue === '' + expected) {
91 return expected;
92 } else {
93 return stringValue;
@@ -84,11 +102,7 @@ export function getValueForProperty(
102 * The third argument is used as a hint of what the expected value is. Some
103 * attributes have multiple equivalent values.
104 */
87 -export function getValueForAttribute(
88 - node: Element,
89 - name: string,
90 - expected: mixed,
91 -): mixed {
105 +export function getValueForAttribute(node, name, expected) {
106 if (__DEV__) {
107 if (!isAttributeNameSafe(name)) {
108 return;
@@ -97,7 +111,7 @@ export function getValueForAttribute(
111 return expected === undefined ? undefined : null;
112 }
113 const value = node.getAttribute(name);
100 - if (value === '' + (expected: any)) {
114 + if (value === '' + expected) {
115 return expected;
116 }
117 return value;
@@ -111,64 +125,84 @@ export function getValueForAttribute(
125 * @param {string} name
126 * @param {*} value
127 */
114 -export function setValueForProperty(
115 - node: Element,
116 - name: string,
117 - value: mixed,
118 - isCustomComponentTag: boolean,
119 -) {
120 - if (shouldSkipAttribute(name, isCustomComponentTag)) {
121 - return;
122 - }
123 - const propertyInfo = isCustomComponentTag ? null : getPropertyInfo(name);
124 - if (shouldTreatAttributeValueAsNull(name, value, isCustomComponentTag)) {
125 - value = null;
126 - }
127 - // If the prop isn't in the special list, treat it as a simple attribute.
128 - if (!propertyInfo) {
129 - if (isAttributeNameSafe(name)) {
130 - const attributeName = name;
131 - if (value == null) {
132 - node.removeAttribute(attributeName);
128 +export function setValueForProperty(node, name, value) {
129 + const propertyInfo = getPropertyInfo(name);
130 +
131 + if (propertyInfo && shouldSetAttribute(name, value)) {
132 + if (shouldIgnoreValue(propertyInfo, value)) {
133 + deleteValueForProperty(node, name);
134 + return;
135 + } else if (propertyInfo.mustUseProperty) {
136 + // Contrary to `setAttribute`, object properties are properly
137 + // `toString`ed by IE8/9.
138 + node[propertyInfo.propertyName] = value;
139 + } else {
140 + const attributeName = propertyInfo.attributeName;
141 + const namespace = propertyInfo.attributeNamespace;
142 + // `setAttribute` with objects becomes only `[object]` in IE8/9,
143 + // ('' + value) makes it output the correct toString()-value.
144 + if (namespace) {
145 + node.setAttributeNS(namespace, attributeName, '' + value);
146 + } else if (
147 + propertyInfo.hasBooleanValue ||
148 + (propertyInfo.hasOverloadedBooleanValue && value === true)
149 + ) {
150 + node.setAttribute(attributeName, '');
151 } else {
134 - node.setAttribute(attributeName, '' + (value: any));
152 + node.setAttribute(attributeName, '' + value);
153 }
154 }
155 + } else {
156 + setValueForAttribute(
157 + node,
158 + name,
159 + shouldSetAttribute(name, value) ? value : null,
160 + );
161 return;
162 }
139 - const {
140 - hasBooleanValue,
141 - hasOverloadedBooleanValue,
142 - mustUseProperty,
143 - } = propertyInfo;
144 - if (mustUseProperty) {
145 - const {propertyName} = propertyInfo;
146 - if (value === null) {
147 - (node: any)[propertyName] = hasBooleanValue ? false : '';
148 - } else {
149 - // Contrary to `setAttribute`, object properties are properly
150 - // `toString`ed by IE8/9.
151 - (node: any)[propertyName] = value;
152 - }
163 +}
164 +
165 +export function setValueForAttribute(node, name, value) {
166 + if (!isAttributeNameSafe(name)) {
167 return;
168 }
155 - // The rest are treated as attributes with special cases.
156 - const {attributeName, attributeNamespace} = propertyInfo;
157 - if (value === null) {
158 - node.removeAttribute(attributeName);
169 + if (value == null) {
170 + node.removeAttribute(name);
171 } else {
160 - let attributeValue;
161 - if (hasBooleanValue || (hasOverloadedBooleanValue && value === true)) {
162 - attributeValue = '';
163 - } else {
164 - // `setAttribute` with objects becomes only `[object]` in IE8/9,
165 - // ('' + value) makes it output the correct toString()-value.
166 - attributeValue = '' + (value: any);
167 - }
168 - if (attributeNamespace) {
169 - node.setAttributeNS(attributeNamespace, attributeName, attributeValue);
172 + node.setAttribute(name, '' + value);
173 + }
174 +}
175 +
176 +/**
177 + * Deletes an attributes from a node.
178 + *
179 + * @param {DOMElement} node
180 + * @param {string} name
181 + */
182 +export function deleteValueForAttribute(node, name) {
183 + node.removeAttribute(name);
184 +}
185 +
186 +/**
187 + * Deletes the value for a property on a node.
188 + *
189 + * @param {DOMElement} node
190 + * @param {string} name
191 + */
192 +export function deleteValueForProperty(node, name) {
193 + const propertyInfo = getPropertyInfo(name);
194 + if (propertyInfo) {
195 + if (propertyInfo.mustUseProperty) {
196 + const propName = propertyInfo.propertyName;
197 + if (propertyInfo.hasBooleanValue) {
198 + node[propName] = false;
199 + } else {
200 + node[propName] = '';
201 + }
202 } else {
171 - node.setAttribute(attributeName, attributeValue);
203 + node.removeAttribute(propertyInfo.attributeName);
204 }
205 + } else {
206 + node.removeAttribute(name);
207 }
208 }
packages/react-dom/src/client/ReactDOMFiberComponent.js
+25 -30
@@ -24,11 +24,7 @@ import setTextContent from './setTextContent';
24 import {listenTo, trapBubbledEvent} from '../events/ReactBrowserEventEmitter';
25 import * as CSSPropertyOperations from '../shared/CSSPropertyOperations';
26 import {Namespaces, getIntrinsicNamespace} from '../shared/DOMNamespaces';
27 -import {
28 - getPropertyInfo,
29 - shouldSkipAttribute,
30 - shouldTreatAttributeValueAsNull,
31 -} from '../shared/DOMProperty';
27 +import {getPropertyInfo, shouldSetAttribute} from '../shared/DOMProperty';
28 import assertValidProps from '../shared/assertValidProps';
29 import {DOCUMENT_NODE, DOCUMENT_FRAGMENT_NODE} from '../shared/HTMLNodeType';
30 import isCustomComponent from '../shared/isCustomComponent';
@@ -318,13 +314,13 @@ function setInitialDOMProperties(
314 }
315 ensureListeningTo(rootContainerElement, propKey);
316 }
317 + } else if (isCustomComponentTag) {
318 + DOMPropertyOperations.setValueForAttribute(domElement, propKey, nextProp);
319 } else if (nextProp != null) {
322 - DOMPropertyOperations.setValueForProperty(
323 - domElement,
324 - propKey,
325 - nextProp,
326 - isCustomComponentTag,
327 - );
320 + // If we're updating to null or undefined, we should remove the property
321 + // from the DOM node instead of inadvertently setting to a string. This
322 + // brings us in line with the same behavior we have on initial render.
323 + DOMPropertyOperations.setValueForProperty(domElement, propKey, nextProp);
324 }
325 }
326 }
@@ -345,13 +341,23 @@ function updateDOMProperties(
341 setInnerHTML(domElement, propValue);
342 } else if (propKey === CHILDREN) {
343 setTextContent(domElement, propValue);
344 + } else if (isCustomComponentTag) {
345 + if (propValue != null) {
346 + DOMPropertyOperations.setValueForAttribute(
347 + domElement,
348 + propKey,
349 + propValue,
350 + );
351 + } else {
352 + DOMPropertyOperations.deleteValueForAttribute(domElement, propKey);
353 + }
354 + } else if (propValue != null) {
355 + DOMPropertyOperations.setValueForProperty(domElement, propKey, propValue);
356 } else {
349 - DOMPropertyOperations.setValueForProperty(
350 - domElement,
351 - propKey,
352 - propValue,
353 - isCustomComponentTag,
354 - );
357 + // If we're updating to null or undefined, we should remove the property
358 + // from the DOM node instead of inadvertently setting to a string. This
359 + // brings us in line with the same behavior we have on initial render.
360 + DOMPropertyOperations.deleteValueForProperty(domElement, propKey);
361 }
362 }
363 }
@@ -959,11 +965,7 @@ export function diffHydratedProperties(
965 }
966 ensureListeningTo(rootContainerElement, propKey);
967 }
962 - } else if (
963 - __DEV__ &&
964 - // Convince Flow we've calculated it (it's DEV-only in this method.)
965 - typeof isCustomComponentTag === 'boolean'
966 - ) {
968 + } else if (__DEV__) {
969 // Validate that the properties correspond to their expected values.
970 let serverValue;
971 let propertyInfo;
@@ -1008,14 +1010,7 @@ export function diffHydratedProperties(
1010 if (nextProp !== serverValue) {
1011 warnForPropDifference(propKey, serverValue, nextProp);
1012 }
1011 - } else if (
1012 - !shouldSkipAttribute(propKey, isCustomComponentTag) &&
1013 - !shouldTreatAttributeValueAsNull(
1014 - propKey,
1015 - nextProp,
1016 - isCustomComponentTag,
1017 - )
1018 - ) {
1013 + } else if (shouldSetAttribute(propKey, nextProp)) {
1014 if ((propertyInfo = getPropertyInfo(propKey))) {
1015 // $FlowFixMe - Should be inferred as not undefined.
1016 extraAttributeNames.delete(propertyInfo.attributeName);
packages/react-dom/src/client/ReactDOMFiberInput.js
+1 -1
@@ -133,7 +133,7 @@ export function updateChecked(element: Element, props: Object) {
133 const node = ((element: any): InputWithWrapperState);
134 const checked = props.checked;
135 if (checked != null) {
136 - DOMPropertyOperations.setValueForProperty(node, 'checked', checked, false);
136 + DOMPropertyOperations.setValueForProperty(node, 'checked', checked);
137 }
138 }
139
packages/react-dom/src/server/DOMMarkupOperations.js
+30 -19
@@ -3,20 +3,30 @@
3 *
4 * This source code is licensed under the MIT license found in the
5 * LICENSE file in the root directory of this source tree.
6 - *
7 - * @flow
6 */
7
8 import {
9 ID_ATTRIBUTE_NAME,
10 ROOT_ATTRIBUTE_NAME,
11 getPropertyInfo,
12 + shouldAttributeAcceptBooleanValue,
13 + shouldSetAttribute,
14 isAttributeNameSafe,
15 - shouldSkipAttribute,
16 - shouldTreatAttributeValueAsNull,
15 } from '../shared/DOMProperty';
16 import quoteAttributeValueForBrowser from './quoteAttributeValueForBrowser';
17
18 +// shouldIgnoreValue() is currently duplicated in DOMPropertyOperations.
19 +// TODO: Find a better place for this.
20 +function shouldIgnoreValue(propertyInfo, value) {
21 + return (
22 + value == null ||
23 + (propertyInfo.hasBooleanValue && !value) ||
24 + (propertyInfo.hasNumericValue && isNaN(value)) ||
25 + (propertyInfo.hasPositiveNumericValue && value < 1) ||
26 + (propertyInfo.hasOverloadedBooleanValue && value === false)
27 + );
28 +}
29 +
30 /**
31 * Operations for dealing with DOM properties.
32 */
@@ -27,11 +37,11 @@ import quoteAttributeValueForBrowser from './quoteAttributeValueForBrowser';
37 * @param {string} id Unescaped ID.
38 * @return {string} Markup string.
39 */
30 -export function createMarkupForID(id: string): string {
40 +export function createMarkupForID(id) {
41 return ID_ATTRIBUTE_NAME + '=' + quoteAttributeValueForBrowser(id);
42 }
43
34 -export function createMarkupForRoot(): string {
44 +export function createMarkupForRoot() {
45 return ROOT_ATTRIBUTE_NAME + '=""';
46 }
47
@@ -42,27 +52,31 @@ export function createMarkupForRoot(): string {
52 * @param {*} value
53 * @return {?string} Markup string, or null if the property was invalid.
54 */
45 -export function createMarkupForProperty(name: string, value: mixed): string {
46 - if (name !== 'style' && shouldSkipAttribute(name, false)) {
47 - return '';
48 - }
49 - if (shouldTreatAttributeValueAsNull(name, value, false)) {
50 - return '';
51 - }
55 +export function createMarkupForProperty(name, value) {
56 const propertyInfo = getPropertyInfo(name);
57 if (propertyInfo) {
58 + if (shouldIgnoreValue(propertyInfo, value)) {
59 + return '';
60 + }
61 const attributeName = propertyInfo.attributeName;
62 if (
63 propertyInfo.hasBooleanValue ||
64 (propertyInfo.hasOverloadedBooleanValue && value === true)
65 ) {
66 return attributeName + '=""';
60 - } else {
67 + } else if (
68 + typeof value !== 'boolean' ||
69 + shouldAttributeAcceptBooleanValue(name)
70 + ) {
71 return attributeName + '=' + quoteAttributeValueForBrowser(value);
72 }
63 - } else {
73 + } else if (shouldSetAttribute(name, value)) {
74 + if (value == null) {
75 + return '';
76 + }
77 return name + '=' + quoteAttributeValueForBrowser(value);
78 }
79 + return null;
80 }
81
82 /**
@@ -72,10 +86,7 @@ export function createMarkupForProperty(name: string, value: mixed): string {
86 * @param {*} value
87 * @return {string} Markup string, or empty string if the property was invalid.
88 */
75 -export function createMarkupForCustomAttribute(
76 - name: string,
77 - value: mixed,
78 -): string {
89 +export function createMarkupForCustomAttribute(name, value) {
90 if (!isAttributeNameSafe(name) || value == null) {
91 return '';
92 }
packages/react-dom/src/shared/DOMProperty.js
+29 -71
@@ -3,24 +3,10 @@
3 *
4 * This source code is licensed under the MIT license found in the
5 * LICENSE file in the root directory of this source tree.
6 - *
7 - * @flow
6 */
7
8 import warning from 'fbjs/lib/warning';
9
12 -type PropertyInfo = {|
13 - attributeName: string,
14 - attributeNamespace: string | null,
15 - propertyName: string,
16 - mustUseProperty: boolean,
17 - hasBooleanValue: boolean,
18 - hasNumericValue: boolean,
19 - hasPositiveNumericValue: boolean,
20 - hasOverloadedBooleanValue: boolean,
21 - hasStringBooleanValue: boolean,
22 -|};
23 -
10 // These attributes should be all lowercase to allow for
11 // case insensitive checks
12 const RESERVED_PROPS = {
@@ -68,7 +54,7 @@ function injectDOMPropertyConfig(domPropertyConfig) {
54 const lowerCased = propName.toLowerCase();
55 const propConfig = Properties[propName];
56
71 - const propertyInfo: PropertyInfo = {
57 + const propertyInfo = {
58 attributeName: lowerCased,
59 attributeNamespace: null,
60 propertyName: propName,
@@ -132,7 +118,7 @@ export const VALID_ATTRIBUTE_NAME_REGEX = new RegExp(
118 const illegalAttributeNameCache = {};
119 const validatedAttributeNameCache = {};
120
135 -export function isAttributeNameSafe(attributeName: string): boolean {
121 +export function isAttributeNameSafe(attributeName) {
122 if (validatedAttributeNameCache.hasOwnProperty(attributeName)) {
123 return true;
124 }
@@ -177,14 +163,12 @@ export function isAttributeNameSafe(attributeName: string): boolean {
163 */
164 export const properties = {};
165
180 -export function shouldSkipAttribute(
181 - name: string,
182 - isCustomComponentTag: boolean,
183 -): boolean {
166 +/**
167 + * Checks whether a property name is a writeable attribute.
168 + * @method
169 + */
170 +export function shouldSetAttribute(name, value) {
171 if (isReservedProp(name)) {
185 - return true;
186 - }
187 - if (isCustomComponentTag) {
172 return false;
173 }
174 if (
@@ -192,69 +176,43 @@ export function shouldSkipAttribute(
176 (name[0] === 'o' || name[0] === 'O') &&
177 (name[1] === 'n' || name[1] === 'N')
178 ) {
195 - return true;
196 - }
197 - return false;
198 -}
199 -
200 -export function isBadlyTypedAttributeValue(
201 - name: string,
202 - value: mixed,
203 - isCustomComponentTag: boolean,
204 -): boolean {
205 - if (isReservedProp(name)) {
179 return false;
180 }
181 + if (value === null) {
182 + return true;
183 + }
184 switch (typeof value) {
209 - case 'function':
210 - // $FlowIssue symbol is perfectly valid here
211 - case 'symbol': // eslint-disable-line
212 - return true;
185 case 'boolean':
214 - break;
186 + return shouldAttributeAcceptBooleanValue(name);
187 + case 'undefined':
188 + case 'number':
189 + case 'string':
190 + case 'object':
191 + return true;
192 default:
193 + // function, symbol
194 return false;
195 }
218 - if (isCustomComponentTag) {
219 - return false;
196 +}
197 +
198 +export function getPropertyInfo(name) {
199 + return properties.hasOwnProperty(name) ? properties[name] : null;
200 +}
201 +
202 +export function shouldAttributeAcceptBooleanValue(name) {
203 + if (isReservedProp(name)) {
204 + return true;
205 }
206 let propertyInfo = getPropertyInfo(name);
207 if (propertyInfo) {
223 - return !(
208 + return (
209 propertyInfo.hasBooleanValue ||
210 propertyInfo.hasStringBooleanValue ||
211 propertyInfo.hasOverloadedBooleanValue
212 );
213 }
214 const prefix = name.toLowerCase().slice(0, 5);
230 - return prefix !== 'data-' && prefix !== 'aria-';
231 -}
232 -
233 -export function shouldTreatAttributeValueAsNull(
234 - name: string,
235 - value: mixed,
236 - isCustomComponentTag: boolean,
237 -): boolean {
238 - if (value === null || typeof value === 'undefined') {
239 - return true;
240 - }
241 - const propertyInfo = getPropertyInfo(name);
242 - if (propertyInfo) {
243 - if (propertyInfo.hasBooleanValue) {
244 - return !value;
245 - } else if (propertyInfo.hasOverloadedBooleanValue) {
246 - return value === false;
247 - } else if (propertyInfo.hasNumericValue && isNaN(value)) {
248 - return true;
249 - } else if (propertyInfo.hasPositiveNumericValue && (value: any) < 1) {
250 - return true;
251 - }
252 - }
253 - return isBadlyTypedAttributeValue(name, value, isCustomComponentTag);
254 -}
255 -
256 -export function getPropertyInfo(name: string): PropertyInfo | null {
257 - return properties.hasOwnProperty(name) ? properties[name] : null;
215 + return prefix === 'data-' || prefix === 'aria-';
216 }
217
218 /**
@@ -266,7 +224,7 @@ export function getPropertyInfo(name: string): PropertyInfo | null {
224 * @param {string} name
225 * @return {boolean} If the name is within reserved props
226 */
269 -export function isReservedProp(name: string): boolean {
227 +export function isReservedProp(name) {
228 return RESERVED_PROPS.hasOwnProperty(name);
229 }
230
packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
+4 -3
@@ -15,7 +15,8 @@ import warning from 'fbjs/lib/warning';
15 import {
16 ATTRIBUTE_NAME_CHAR,
17 isReservedProp,
18 - isBadlyTypedAttributeValue,
18 + shouldAttributeAcceptBooleanValue,
19 + shouldSetAttribute,
20 } from './DOMProperty';
21 import isCustomComponent from './isCustomComponent';
22 import possibleStandardNames from './possibleStandardNames';
@@ -190,7 +191,7 @@ if (__DEV__) {
191
192 if (
193 typeof value === 'boolean' &&
193 - isBadlyTypedAttributeValue(name, value, false)
194 + !shouldAttributeAcceptBooleanValue(name)
195 ) {
196 if (value) {
197 warning(
@@ -234,7 +235,7 @@ if (__DEV__) {
235 }
236
237 // Warn when a known attribute is a bad type
237 - if (isBadlyTypedAttributeValue(name, value, false)) {
238 + if (!shouldSetAttribute(name, value)) {
239 warnedProperties[name] = true;
240 return false;
241 }