Skip to content

Commit

Permalink
[BUGFIX beta] Add assertion when calling this.$() in a tagless view.
Browse files Browse the repository at this point in the history
  • Loading branch information
rwjblue committed Feb 25, 2015
1 parent 25e679b commit 871aa64
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/ember-views/lib/views/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ var View = CoreView.extend(
@return {jQuery} the jQuery object for the DOM node
*/
$: function(sel) {
Ember.assert('You cannot access this.$() on a component with `tagName: \'\'` specified.', this.tagName !== '');
return this.currentState.$(this, sel);
},

Expand Down
27 changes: 17 additions & 10 deletions packages/ember-views/tests/views/view/jquery_test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { get } from "ember-metal/property_get";
import run from "ember-metal/run_loop";
import EmberView from "ember-views/views/view";
import { runAppend, runDestroy } from "ember-runtime/tests/utils";

var view;
QUnit.module("EmberView#$", {
Expand All @@ -11,15 +11,11 @@ QUnit.module("EmberView#$", {
}
}).create();

run(function() {
view.append();
});
runAppend(view);
},

teardown: function() {
run(function() {
view.destroy();
});
runDestroy(view);
}
});

Expand All @@ -29,9 +25,7 @@ QUnit.test("returns undefined if no element", function() {
equal(view.$(), undefined, 'should return undefined');
equal(view.$('span'), undefined, 'should undefined if filter passed');

run(function() {
view.destroy();
});
runDestroy(view);
});

QUnit.test("returns jQuery object selecting element if provided", function() {
Expand All @@ -57,3 +51,16 @@ QUnit.test("returns empty jQuery object if filter passed that does not match ite
equal(jquery.length, 0, 'view.$(body) should have no elements');
});

QUnit.test("asserts for tagless views", function() {
var view = EmberView.create({
tagName: ''
});

runAppend(view);

expectAssertion(function() {
view.$();
}, /You cannot access this.\$\(\) on a component with `tagName: \'\'` specified/);

runDestroy(view);
});

8 comments on commit 871aa64

@kellyselden
Copy link
Member

Choose a reason for hiding this comment

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

This change breaks a library I use. I extracted out the breaking bit http://emberjs.jsbin.com/mukikojusa/2/edit?html,js,output

@stefanpenner
Copy link
Member

Choose a reason for hiding this comment

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

@kellyselden that break appears expected. That library must check if the view is tagless or not. I have no idea what $() is supposed to be for a tagless view, it is basically just an invalid case.

An add-on that adds this to every view seems dubious at best.

@kellyselden
Copy link
Member

Choose a reason for hiding this comment

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

@stefanpenner after researching more, I agree with the change and the unexpectedness of using it.

If you're curious, the library is ember-animate. It manages to function with it so maybe it was unused code.

@john-kurkowski
Copy link

Choose a reason for hiding this comment

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

In an Ember CLI moduleForComponent, this.render() is failing this assertion for my tagless component. I guess Ember CLI's test helpers need to be updated for this?

@rwjblue
Copy link
Member Author

@rwjblue rwjblue commented on 871aa64 Mar 29, 2015 via email

Choose a reason for hiding this comment

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

@Turtleguyy
Copy link

Choose a reason for hiding this comment

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

@stefanpenner, sorry I'm a bit confused. Shouldn't all view's be using a div tag unless otherwise specified? In @kellyselden's JS Bin, he's never explicitly saying tagName: '', so why does that break?

@rwjblue
Copy link
Member Author

@rwjblue rwjblue commented on 871aa64 Apr 9, 2015

Choose a reason for hiding this comment

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

OutletView is tagless

@Turtleguyy
Copy link

Choose a reason for hiding this comment

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

Hey look at that! So he is. Sneaky little bugger.

Please sign in to comment.