Attributes: Fixed Coding Convention #3221
Conversation
Properties should check undefined using `object.prop === undefined`. https://contribute.jquery.org/style-guide/js/#type-checks
| @@ -34,7 +34,7 @@ jQuery.extend( { | |||
| } | |||
|
|
|||
| // Fallback to prop when attributes are not supported | |||
| if ( typeof elem.getAttribute === "undefined" ) { | |||
| if ( elem.getAttribute === undefined ) { | |||
There was a problem hiding this comment.
This was intentional, but probably not needed anymore. @jquery/core Does anyone remember at what point IE stopped automatically invoking the method on mere reference?
There was a problem hiding this comment.
Either IE7 or IE8, so we should be in the clear.
There was a problem hiding this comment.
It was only certain properties in IE8, but I think we're safe now. We could probably do just...
if ( !elem.getAttribute ) {
There was a problem hiding this comment.
Wouldn't it violate our code style?
There was a problem hiding this comment.
Also, pretty sure we added tests for that, so if they pass we should definitely be in the clear
There was a problem hiding this comment.
Wouldn't it violate our code style?
Sure, we can change it, tho. Those styles were decided based on what we needed to do for old IE.
There was a problem hiding this comment.
I do, as long as we no longer have to use typeof.
There was a problem hiding this comment.
To conclude: we still need typeof in some places for all IE versions as it still has the auto-invoking problem for some host objects: https://github.com/jquery/jquery/blob/e4fd41f/src/manipulation/getAll.js#L10
|
Added correspondent ticket for eslint config - jquery/eslint-config-jquery#5 |
|
Since we should make a sweep across the whole repo, and the check will probably not look like the one proposed here, I'll close this PR. Added to Roadmap. |
Summary
Properties should check undefined using
object.prop === undefined.https://contribute.jquery.org/style-guide/js/#type-checks
Checklist
Mark an
[x]for completed items, if you're not sure leave them unchecked and we can assist.Thanks! Bots and humans will be around shortly to check it out.