From e3d16baef2bb9a55bfcd6601405271a3e9649b45 Mon Sep 17 00:00:00 2001 From: Kevin Linsley Date: Thu, 25 May 2017 10:37:38 -0500 Subject: [PATCH 1/3] Custom boolean attributes are not coerced to strings --- src/renderers/dom/shared/DOMPropertyOperations.js | 5 ++++- .../dom/shared/__tests__/DOMPropertyOperations-test.js | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 26e9e6c59316..43e7d5797908 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -170,7 +170,6 @@ var DOMPropertyOperations = { DOMPropertyOperations.setValueForAttribute(node, name, value); return; } - if (__DEV__) { var payload = {}; payload[name] = value; @@ -188,6 +187,10 @@ var DOMPropertyOperations = { } if (value == null) { node.removeAttribute(name); + } else if (value === true || value === 'true') { + node.setAttribute(name, ''); + } else if (value === false || value === 'false') { + node.removeAttribute(name); } else { node.setAttribute(name, '' + value); } diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 5fcc92a825aa..14dd91a27e20 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -142,7 +142,7 @@ describe('DOMPropertyOperations', () => { }); }); - describe('setValueForProperty', () => { + describe.only('setValueForProperty', () => { var stubNode; var stubInstance; @@ -203,6 +203,13 @@ describe('DOMPropertyOperations', () => { expect(stubNode.getAttribute('role')).toBe(''); }); + it.only('should set values as boolean attributes', () => { + DOMPropertyOperations.setValueForProperty(stubNode, 'data-foo', true); + expect(stubNode.getAttribute('data-foo')).toBe(''); + DOMPropertyOperations.setValueForProperty(stubNode, 'data-foo', false); + expect(stubNode.getAttribute('data-foo')).toBe(null); + }); + it('should not remove empty attributes for special properties', () => { stubNode = document.createElement('input'); ReactDOMComponentTree.precacheNode(stubInstance, stubNode); From cf9ee44b586f57e0045fc8d9fefe3df0ba0e0b5f Mon Sep 17 00:00:00 2001 From: Kevin Linsley Date: Thu, 25 May 2017 12:07:20 -0500 Subject: [PATCH 2/3] updating boolean attribute docs --- docs/docs/jsx-in-depth.md | 4 ++-- src/renderers/dom/shared/DOMPropertyOperations.js | 5 +++-- .../dom/shared/__tests__/DOMPropertyOperations-test.js | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/docs/jsx-in-depth.md b/docs/docs/jsx-in-depth.md index 282c199d5573..94143dd37cef 100644 --- a/docs/docs/jsx-in-depth.md +++ b/docs/docs/jsx-in-depth.md @@ -218,9 +218,9 @@ When you pass a string literal, its value is HTML-unescaped. So these two JSX ex This behavior is usually not relevant. It's only mentioned here for completeness. -### Props Default to "True" +### Props Default to Empty String -If you pass no value for a prop, it defaults to `true`. These two JSX expressions are equivalent: +If you pass no value for a prop, it defaults to ''. These two JSX expressions are equivalent: ```js diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 43e7d5797908..e8dd5a1a6ade 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -170,6 +170,7 @@ var DOMPropertyOperations = { DOMPropertyOperations.setValueForAttribute(node, name, value); return; } + if (__DEV__) { var payload = {}; payload[name] = value; @@ -187,9 +188,9 @@ var DOMPropertyOperations = { } if (value == null) { node.removeAttribute(name); - } else if (value === true || value === 'true') { + } else if (value === true) { node.setAttribute(name, ''); - } else if (value === false || value === 'false') { + } else if (value === false) { node.removeAttribute(name); } else { node.setAttribute(name, '' + value); diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 14dd91a27e20..b64a5273d677 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -142,7 +142,7 @@ describe('DOMPropertyOperations', () => { }); }); - describe.only('setValueForProperty', () => { + describe('setValueForProperty', () => { var stubNode; var stubInstance; @@ -203,7 +203,7 @@ describe('DOMPropertyOperations', () => { expect(stubNode.getAttribute('role')).toBe(''); }); - it.only('should set values as boolean attributes', () => { + it('should set values as boolean attributes', () => { DOMPropertyOperations.setValueForProperty(stubNode, 'data-foo', true); expect(stubNode.getAttribute('data-foo')).toBe(''); DOMPropertyOperations.setValueForProperty(stubNode, 'data-foo', false); From c1fb10057efbbffe9930d4664d157b6a296df73f Mon Sep 17 00:00:00 2001 From: Kevin Linsley Date: Thu, 25 May 2017 12:23:43 -0500 Subject: [PATCH 3/3] run fiber --- scripts/fiber/tests-passing.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 52ac61aeb1fa..461479c2f346 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -800,6 +800,7 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should set values as namespace attributes if necessary * should set values as boolean properties * should convert attribute values to string first +* should set values as boolean attributes * should not remove empty attributes for special properties * should remove for falsey boolean properties * should remove when setting custom attr to null