Skip to content

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

Closed
wants to merge 2 commits into from

Conversation

dmethvin
Copy link
Member

@dmethvin dmethvin commented Jan 9, 2015

this[0] = elem;
this.length = 1;
Copy link
Member

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

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. 😄

Copy link
Member

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" );
Copy link
Member

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"

Copy link
Member

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.

Copy link
Member

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 :-(

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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

Copy link
Member

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.

@gibson042
Copy link
Member

👍

dmethvin added a commit that referenced this pull request Jan 12, 2015
@dmethvin dmethvin closed this in 0ea8c32 Jan 12, 2015
@dmethvin dmethvin deleted the 1908-context-selector branch January 24, 2015 21:15
@markelog markelog mentioned this pull request Nov 16, 2015
@markelog markelog mentioned this pull request Dec 22, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Remove context and selector properties
4 participants