AmpersandJS / ampersand-view

A smart base view for Backbone apps, to make it easy to bind collections and properties to the DOM.
http://ampersandjs.com
MIT License
92 stars 39 forks source link

Error when removing Views: _subviews is undefined #66

Closed zeorin closed 10 years ago

zeorin commented 10 years ago

When calling a View's remove() method, ampersand-view tries to remove subViews, too. Even if they don't exist. This causes an error much further on when Underscore's flatten() tries to flatten a variable which it expects to be an array, but is actually undefined. In Chrome, the error I get is:

Uncaught TypeError: Cannot read property 'length' of undefined

Changing line 134 from

_.chain(this._subviews).flatten().invoke('remove');

to

if (this._subviews) _.chain(this._subviews).flatten().invoke('remove');

Fixes the error.

However, I don't know if this is perhaps indicative of a deeper bug, like perhaps Views ought to be initialized with an empty array _subviews property instead. If not, I'm happy to create a pull request.

latentflip commented 10 years ago

Interesting,

My guess is you are using the recently released underscore 1.7.0 in your project? AmpersandView is pinned to 1.6.0 but npm will still use a later version if it was installed in your project.

It would seem that the flatten behaviour has changed (I just reported it to underscore).

zeorin commented 10 years ago

Yes, I'm using 1.7.0, even though my underscore depency declaration in package.json is the same as the one in AmpersandView's package.json:

"underscore": "^1.6.0"

I understand that's the expected behaviour, however. How did you nevertheless pin it? Is it because ampersand-collection-view's Underscore.js is pinned (because it's a dependency of ampersand-view)?

I also noticed that your underscore version requirements for the different Ampersand modules are not consistent. I'm not sure if that is by design:

latentflip commented 10 years ago

Ah, it should be pinned exactly since underscore doesn't respect semver. They should all be the same, but we've been discussing getting away from underscore for this exact reason so had delayed from the short term fix.

Pin your app to eXactly 1.6 in the short term and you should be okay.

Philip Roberts &yet

On 26 Sep 2014, at 10:35, Xandor Schiefer notifications@github.com wrote:

Yes, I'm using 1.7.0, even though my underscore depency declaration in package.json is the same as the one in AmpersandView's package.json:

"underscore": "^1.6.0" I understand that's the expected behaviour, however. How did you nevertheless pin it? Is it because ampersand-collection-view's Underscore.js is pinned (because it's a dependency of ampersand-view)?

I also noticed that your underscore version requirements for the different Ampersand modules are not consistent. I'm not sure if that is by design:

ampersand-sync: "underscore": "~1.6.0" ampersand-collection-underscore-mixin: "underscore": "^1.6.0" ampersand-router: "underscore": "1.6.0" ampersand-state: "underscore": "^1.6.0" ampersand-model: "underscore": "^1.6.0" ampersand-view: "underscore": "^1.6.0" ampersand-collection-view: "underscore": "1.6.0" — Reply to this email directly or view it on GitHub.

zeorin commented 10 years ago

Pinning it in my app works like a charm!

latentflip commented 10 years ago

@zeorin I've just checked and the latest versions of ampersand-view and ampersand-collection-view are all already exactly pinned to 1.6.0. So I'm guessing you created this app a while ago, or your ampersand-cli needs an update. Either way, you may wish to look at updating your apps dependencies.

Thanks, Phil