this.$(someElement) Doesn't Work #1038

Closed
machineghost opened this Issue Feb 22, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@machineghost

Somewhere between 0.5 and 0.9 Backbone changed the definition of the delegator (this.$()) from:

return $(selector, this.el);

to:

return this.$el.find(selector);

Now as the jQuery site itself says:

"Selector context is implemented with the .find() method; therefore, $('li.item-ii').find('li') is equivalent to $('li', 'li.item-ii')."

so that change should be perfectly safe, right? Well, it is, but only:
A) for jQuery >= 1.6
B) if the selector is a string

If however you try to do:
var model = new Backbone.Model.extend({
....someFunc: function() {
........var elem = document.getElementById("foo");
........var $elem = this.$(elem);
....}
}) ()
before jQuery 1.6, it won't work, because find isn't expecting to take an element (why would you try and find something you already have?).

Now similarly, in Backbone it makes no sense to do this.$(elem) when you can just as easily do $(elem) (and the latter works). But sometimes you might have a variable that could be a string or could be an element ... or you might just have some legacy code that does the former, and it's a pain to change.

All of this can be solved ... well in two ways (that I see):

  1. Add a line to the Backbone documentation that says something to the effect of "While Backbone 0.9 works with all versions of jQuery, jQuery 1.6 or greater is recommended; using earlier versions of jQuery may cause problems in certain corner cases."
  2. Add a quick type check to the delegate function, and use the old style if the selector isn't a string:
    $: function(selector) {
    .... if (typeof selector != "string") return $(selector, this.$el);
    ....return this.$el.find(selector);
    },
@braddunbar

This comment has been minimized.

Show comment Hide comment
@braddunbar

braddunbar Feb 22, 2012

Collaborator

It's equivalent to running: $(selector, this.el)

I've also been bitten by this change. The docs should definitely be updated since the above is no longer true. An item in the upgrade notes would be nice as well. I'll do a quick write up.

Collaborator

braddunbar commented Feb 22, 2012

It's equivalent to running: $(selector, this.el)

I've also been bitten by this change. The docs should definitely be updated since the above is no longer true. An item in the upgrade notes would be nice as well. I'll do a quick write up.

@braddunbar

This comment has been minimized.

Show comment Hide comment
@braddunbar

braddunbar Feb 22, 2012

Collaborator

Thanks for pointing this out! Addressed in #1040.

Collaborator

braddunbar commented Feb 22, 2012

Thanks for pointing this out! Addressed in #1040.

braddunbar pushed a commit to braddunbar/backbone that referenced this issue Feb 27, 2012

Merge pull request #1040 from braddunbar/$-docs
Fixes #1038 - Document changes to `view.$`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment