- Notifications
You must be signed in to change notification settings - Fork20.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Or just for the consistency withhttps://github.com/jquery/jquery/pull/2000/files#diff-85f162ae43172ebe26b51a4d1c5d4eddR94
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: thepresence 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:
functiontest(){}newtest().constructor.name// test// i.e. we could dofunctionjQuery(){}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 thinksomeobject.fn.version
would be that ambiguous but it's also available viasomeobject.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 tonodeType
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 beundefined
for objects that arenot jQuery collections—can be cast as a boolean for type checking, just like castingnodeType
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.
👍 |
Fixesgh-1908
Refjquery/jquery-migrate#79