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

Extending a Model overwrites parseModels() #1246

Closed
akagomez opened this Issue Oct 1, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@akagomez
Contributor

akagomez commented Oct 1, 2014

If you do something like...

var GenericModel = can.Model.extend({
  findAll: function () {
    ...
    var models = this.parseModels(data); 
    ...
  }, 
  parseModels: function () {
    // Never gets called
  }
}, {});

var SpecificModel = GenericModel.extend({}, {}); 
SpecificModel.findAll({}); 

...parseModels() never gets called.

Here is a Fiddle and a test demonstrating the issue.

I suspected the issue might be here: model.js#L1235-L1239, where the Model checks to see if model(s) or parseModel(s) were defined as a static property. But it doesn't check if they're defined on the Constructor.

If all you do is add && !base[name] && !base[parseName] to models.js#L1237 parseModels won't be overwritten. But then parseModels is called twice. The first time its passed a raw object. The second time, a can.Map.

@akagomez akagomez added the bug label Oct 1, 2014

@DVSoftware

This comment has been minimized.

Show comment
Hide comment
@DVSoftware

DVSoftware Oct 8, 2014

Contributor

We are also experiencing this issue

Contributor

DVSoftware commented Oct 8, 2014

We are also experiencing this issue

@akagomez

This comment has been minimized.

Show comment
Hide comment
@akagomez

akagomez Oct 8, 2014

Contributor

While mucking with this I learned that parseModels shouldn't be called directly. My bad.

This is still an issue though as shown in my test, which doesn't call parseModels directly.

I also learned that you can work around this issue by defining parseModels in init.

The problem then becomes that parseModels is called twice. The same is true for parseModel. See issue #1253.

The second time parseModels is called it's passed a can.List instance. My workaround for this was to check the instance type of the argument passed to parseModels.

Here's a demo of how I hacked my way around both issues: http://jsfiddle.net/qYdwR/2194/

Contributor

akagomez commented Oct 8, 2014

While mucking with this I learned that parseModels shouldn't be called directly. My bad.

This is still an issue though as shown in my test, which doesn't call parseModels directly.

I also learned that you can work around this issue by defining parseModels in init.

The problem then becomes that parseModels is called twice. The same is true for parseModel. See issue #1253.

The second time parseModels is called it's passed a can.List instance. My workaround for this was to check the instance type of the argument passed to parseModels.

Here's a demo of how I hacked my way around both issues: http://jsfiddle.net/qYdwR/2194/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment