Skip to content
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

Parse() not applying to fetchAll. Only seems to work with fetch(). #962

Closed
jpstone opened this issue Oct 21, 2015 · 10 comments
Closed

Parse() not applying to fetchAll. Only seems to work with fetch(). #962

jpstone opened this issue Oct 21, 2015 · 10 comments

Comments

@jpstone
Copy link

jpstone commented Oct 21, 2015

So when I run a .fetch({id: x}), the result has the columns in the camelCase I want. But when I do a fetchAll, they are in snake_case.

Here's my extension of the Model class:

var proto = bookshelf.Model.prototype;

var Model = bookshelf.Model.extend({
  constructor: function () {
    proto.constructor.apply(this, arguments);
    this.parse = function (attr) {
      return _.reduce(attr, function (record, val, key) {
        record[_.camelCase(key)] = val;
        return record;
      }, {});
    },
    this.format = function (attr) {
      return _.reduce(attr, function (record, val, key) {
        record[_.snakeCase(key)] = val;
        return record;
      }, {});
    }
  }
});

bookshelf.Model = Model;

Is this supposed to work with fetchAll too, or is it meant to ignore fetchAll? Any suggestions on how to work around this or extend the Model class so it works?

@jpstone jpstone changed the title Parse() and format() not applying to fetchAll. Only seems to work with fetch(). Parse() not applying to fetchAll. Only seems to work with fetch(). Oct 21, 2015
@ricardograca
Copy link
Member

Why aren't you defining parse and format on extend instead? I'm almost certain that is the problem here.

@jpstone
Copy link
Author

jpstone commented Oct 21, 2015

So I can stay DRY. I'm simply following the recommendation from this issue:

#828 (comment)

If that's the problem, why does it work when doing fetch(), but not fetchAll()?

@ricardograca
Copy link
Member

You seem to be causing some chaos with that approach and it's not exactly what it says on the issue you linked to.

If you want to have a base model from which all others inherit from, take a closer look at what the example is actually doing. It's nothing like what you have here. Also, you need the registry plugin, so make sure it's being loaded before doing anything else.

Tip: right click on the date link on a comment to get a link to the actual comment.

@jpstone
Copy link
Author

jpstone commented Oct 21, 2015

I'm not extending the model as a plugin--I'm just including it in my bookshelf.js config file and exporting bookshelf. So the Model class is already extended before anything actually uses it. I don't see the difference between doing this and doing it as a plugin...am I incorrect in that thought? If so, how?

Again, the parse method works fine with .fetch(), so the extended Model appears to be replicating properly. It just fails with fetchAll(). What can explain this?

Can you be more specific into what you think is wrong with my implementation?

@ricardograca
Copy link
Member

I don't know since I'm not seeing you're entire code, and I'm not that knowledgeable in how Bookshelf works internally, but you're approach doesn't seem right.

My guess is that constructor() isn't called when a model is instantiated as part of a collection, but if there's a parse method on the model's prototype, then that gets called (that's why no one has complained about this issue before). Since you're only adding the parse and format methods on the constructor method, instead of defining them right away on the extend call, your methods never get added to the model instance.

@jpstone
Copy link
Author

jpstone commented Oct 21, 2015

If that's the case, I wouldn't expect it to work at all. But it works for fetch() just fine...

@jpstone
Copy link
Author

jpstone commented Oct 21, 2015

So I did it the way the docs recommended by moving my extension of the Model class to a separate file and registering it as a plugin.

Exact same behavior.

@ricardograca
Copy link
Member

That wasn't the problem. The problem was setting format and parse on constructor(). Did you also take care of that?

@jpstone
Copy link
Author

jpstone commented Oct 21, 2015

Haha yeah, I went ahead and did that before your comment just now. That resolved it.

Thanks for your help!

@jpstone jpstone closed this as completed Oct 21, 2015
@ricardograca
Copy link
Member

No problem. Now we know that constructor() isn't called when a model is instantiated as part of a collection. At least that's what it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants