Null attributes #1516
Null attributes #1516
Conversation
jQuery now returns `null` for empty attributes instead of `undefined`
jQuery now returns `null` for empty attributes instead of `undefined`.
jQuery now returns `null` for empty attributes instead of `undefined`.
jQuery now returns `null` for empty attributes instead of `undefined`.
jQuery now returns `null` for empty attributes instead of `undefined`.
|
Looks good. Out of curiosity how did you find the files to change? I'm assuming failing tests? This seems like something that has the potential to break things unexpectedly. |
|
Yup, failing tests. No idea what else will break in the wild. The core team decided to just do it and see what happens. See jquery/jquery#2118. |
|
Code looks good, no actual testing. I agree with TJ that this is a potentially very dangerous change. Most non-frontend dev are used to always use strict comparison. |
|
What's potentially dangerous? The core change or the UI change? |
|
@timmywil said to bring this up in the next core meeting. The fact that this change broke UI might cause them to back it out. |
|
Just to clarify I meant that the Core change is potentially dangerous. |
|
Ok, that's what I thought. I just wanted to make sure. |
|
What TJ said. |
| @@ -305,11 +305,11 @@ window.domEqual = function( selector, modifier, message ) { | |||
| result = {}; | |||
| $.each( properties, function( index, attr ) { | |||
| var value = elem.prop( attr ); | |||
| result[ attr ] = value !== undefined ? value : ""; | |||
| result[ attr ] = value != null ? value : ""; | |||
There was a problem hiding this comment.
This strict comparison was fine as-is; .prop lacks the DOM semantics that urged us to try changing .attr.
There was a problem hiding this comment.
And looking at the broader context, domEqual might still work if you stopped doing this empty-string normalization altogether and just assigned results from .prop and .attr directly.
There was a problem hiding this comment.
We do the normalization because of space delimited attributes, the most common one being class. The attribute will start out undefined, but after elem.addClass( "foo" ).removeClass( "foo" ), the attribute will be an empty string. We consider this equivalent.
For the property check, the aim was just consistency in the logic. I don't think we've ever consider null and undefined to be non-equivalent in our check.
jQuery now returns `null` for empty attributes instead of `undefined` Ref gh-1516
jQuery now returns `null` for empty attributes instead of `undefined`. Ref gh-1516
jQuery now returns `null` for empty attributes instead of `undefined`. Ref gh-1516
jQuery now returns `null` for empty attributes instead of `undefined`. Ref gh-1516
jquery/jquery@aaeed53