-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Core: Remove deprecated context and selector properties #2000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
this[0] = elem; | ||
this.length = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would save more bytes that way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seemed strange to set the length before the element. I pulled several of the changes from @gibson042 's older PR because they just seemed so right. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
equal( jQuery("#absolute-1").offset(coords).selector, "#absolute-1", "offset(coords) returns jQuery object" ); | ||
equal( jQuery("#non-existent").offset(coords).selector, "#non-existent", "offset(coords) with empty jQuery set returns jQuery object" ); | ||
equal( jQuery("#absolute-1").offset(undefined).selector, "#absolute-1", "offset(undefined) returns jQuery object (#5571)" ); | ||
equal( jQuery("#absolute-1").offset(coords).jquery, jQuery.fn.jquery, "offset(coords) returns jQuery object" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always wondered why this property was named "jquery", not "version"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's killing two birds with one stone: the presence of the property identifies an object as a jQuery collection as opposed to Array/NodeList/Prototype/Zepto/etc., and the value specifies which version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's killing two birds with one stone
Yeah! Or "a bird in the hand is worth two in the bush"
This property indeed serve for two complitly different things, but i don't think that's good, i think these should be exposed explicitly then otherwise. Besides, i'm not sure that was an original intent.
the presence of the property identifies an object as a jQuery
This could always be done by other means, like:
function test() {}
new test().constructor.name // test
// i.e. we could do
function jQuery() {}
jQuery.prototype.constructor = jQuery;
$().constructor.name // "jQuery"
Or other countless ways to do it. Not saying we should do something about it, it might be too late, like with $.each vs $.map
callback arguments case :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, it's too late.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until I looked at the old PR, I had just planned to see if the .selector
property was present but the change by @gibson042 seemed like a slightly better assertion.
I guess .version
was avoided since it's a pretty common name. You wouldn't think someobject.fn.version
would be that ambiguous but it's also available via someobject.prototype.version
so maybe that caused concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, it's too late.
Isn't that frustrating? We can't improve something because people are dependent upon it.
I guess .version was avoided since it's a pretty common name.
If i would do it today, i would called it "version" and provided other ways to identify jquery object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property indeed serve for two complitly different things, but i don't think that's good, i think these should be exposed explicitly then otherwise.
The purposes aren't completely different; they're analogous to nodeType
checks in that both properties are considered to have a universal meaning regardless of where they appear. jquery
specifies the version of jQuery used to construct a collection, which—since it must be undefined
for objects that are not jQuery collections—can be cast as a boolean for type checking, just like casting nodeType
to boolean checks for DOM nodes. That said, though, there might well have been a better choice for the name, but it's definitely too late to change.
👍 |
Fixes gh-1908
Ref jquery/jquery-migrate#79