fnTest is way too broad

[philfreo]

I was trying to upgrade from the original/old version of backbone-super to the latest and haven't been able to because fnTest is way too specific in simply looking for the string _super appearing in the function.

So if I have a simple method like this:

    var _super = Backbone.RelationalModel;
    var Model = _super.extend({
        toContextJSON: function() {
            var data = _super.prototype.toJSON.call(this);
            // ...
        }
    });

Backbone-Super throws an error.

If I rename _super to be a different variable name, it stops complaining. This is a bug.

[michaelBenin]

How are you using this library? Globals? AMD? CommonJS? Something else?

What order are you loading in Backbone.RelationalModel and backbone-super? I'm guessing that BBRM extends off of Backbone Model and if it is than it should get the this._super method if backbone-super is loaded prior.

Just looking at your example code that looks like it would error even without backbone-super.

You can see here that this script doesn't modify Backbone.RelationalModel:
https://github.com/lukasolson/backbone-super/blob/master/backbone-super/backbone-super.js#L26

You bring a good point, we should allow extending other data types or supply a method that could do that. Or add a check for Backbone.RelationModel, then update the readme with it's usage.

We could add a check for Backbone.RelationalModel - What do you think @lukasolson?

[philfreo]

RequireJS/AMD

I don't expect Backbone.RelationalModel to have a this._super() -- my point is that if I'm using a local variable called _super then Backbone-Super is thinking that I'm trying to call this._super() and I'm not.

So then I get an error on Model subclasses saying Super does not implement this method: toContextJSON but if I simply rename _super to _parent everything works.

[michaelBenin]

OK I understand what you're saying now. Yes this looks like a legit bug.

[michaelBenin]

@philfreo if you could setup a test in this repo or possibly a jsbin/jsfiddle demonstrating this issue that would be great.

[philfreo]

Don't have time right now, will try to at some point.

But reproducing it is pretty simple - it's basically just the code above and then calling obj.toContextJSON() on an instance

[michaelBenin]

Hey @philfreo ~

I'm having trouble setting up a repro case:
http://jsbin.com/vipugowoda/1/

[philfreo]

Your jsbin shows the problem for me!

Open the console...

[michaelBenin]

[michaelBenin]

I'm in Chrome what browser are you in?

[philfreo]

To be honest jsbin seems really finicky to me and I can only get it to happen sometimes / I have to adjust the code slightly to get it to run anything. This jsfiddle shows the error for me every time in Chrome and Safari.

http://jsfiddle.net/w2g21gu3/

[michaelBenin]

http://jsbin.com/titagapifu/1/edit?html,js,console

[michaelBenin]

I definitely see this now.