Merge 0c3c03a into 81ac3c4

preactjs · Oct 16, 2019 · 4b390e2 · 4b390e2
2 parents 81ac3c4 + 0c3c03a
commit 4b390e2
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 7 deletions.
diff --git a/src/diff/index.js b/src/diff/index.js
@@ -240,7 +240,7 @@ function diffElementNodes(dom, newVNode, oldVNode, context, isSvg, excessDomChil
 			if (oldProps === EMPTY_OBJ) {
 				oldProps = {};
 				for (let i=0; i<dom.attributes.length; i++) {
-					oldProps[dom.attributes[i].name] = dom.attributes[i].value;
+					oldProps[dom.attributes[i].name] = dom.attributes[i].value || undefined;
 				}
 			}
 

diff --git a/src/diff/props.js b/src/diff/props.js
@@ -14,7 +14,7 @@ export function diffProps(dom, newProps, oldProps, isSvg, hydrate) {
 	let i;
 
 	for (i in oldProps) {
-		if (!(i in newProps)) {
+		if (newProps[i] === undefined) {
 			setProperty(dom, i, null, oldProps[i], isSvg);
 		}
 	}
@@ -96,7 +96,19 @@ function setProperty(dom, name, value, oldValue, isSvg) {
 		&& !isSvg
 		&& (name in dom)
 	) {
-		dom[name] = value==null ? '' : value;
+		if (value==null) {
+			// This condition here guards against modifying a unonctrolled input's value
+			if (name!=='value' && name!=='checked') {
+				// Set the value to empty string first
+				dom[name] = '';
+
+				// And remove the attribute entirely
+				dom.removeAttribute(name == 'className' ? 'class' : name);
+			}
+		}
+		else {
+			dom[name] = value;
+		}
 	}
 	else if (typeof value!=='function' && name!=='dangerouslySetInnerHTML') {
 		if (name!==(name = name.replace(/^xlink:?/, ''))) {

diff --git a/test/browser/render.test.js b/test/browser/render.test.js
@@ -301,11 +301,11 @@ describe('render()', () => {
 
 			test('3');
 			test(null);
-			expect(scratch.innerHTML).to.equal('<div><input><table border=""></table></div>', 'for null');
+			expect(scratch.innerHTML).to.equal('<div><input><table></table></div>', 'for null');
 
 			test('4');
 			test(undefined);
-			expect(scratch.innerHTML).to.equal('<div><input><table border=""></table></div>', 'for undefined');
+			expect(scratch.innerHTML).to.equal('<div><input><table></table></div>', 'for undefined');
 		});
 	}
 
@@ -481,7 +481,7 @@ describe('render()', () => {
 			);
 
 			render(<App />, scratch);
-			expect(scratch.innerHTML).to.equal('<div class=""><span>Bye</span></div>');
+			expect(scratch.innerHTML).to.equal('<div><span>Bye</span></div>');
 		});
 
 		it('should remove class attributes', () => {
@@ -495,7 +495,7 @@ describe('render()', () => {
 			expect(scratch.innerHTML).to.equal('<div class="hi"><span>Bye</span></div>');
 
 			render(<App />, scratch);
-			expect(scratch.innerHTML).to.equal('<div class=""><span>Bye</span></div>');
+			expect(scratch.innerHTML).to.equal('<div><span>Bye</span></div>');
 		});
 
 		it('should remove old styles', () => {
@@ -1017,6 +1017,13 @@ describe('render()', () => {
 		expect(scratch.innerHTML).to.equal('<div>foo</div>');
 	});
 
+	it('should remove old attributes that are undefined', () => {
+		render(<a href={'/'} />, scratch);
+		render(<a />, scratch);
+
+		expect(scratch.firstElementChild.hasAttribute('href')).to.be.false;
+	});
+
 	// see preact/#1327
 	it('should not reuse unkeyed components', () => {
 		class X extends Component {