ianhaggerty / backbone-sails

A plugin for Backbone that makes integrating with Sails JS easier. Both leverage a model-view architecture, it's only natural to have them talk to each other.
MIT License
7 stars 1 forks source link

Watched collection doesn't update #4

Closed hardchor closed 9 years ago

hardchor commented 9 years ago

Hey Oscar (Ian?),

First of all, thanks for such a great plugin and the effort you put into documentation!

I'm experiencing an issue when updating a fetched (and subscribed) collection (PersonCollection containing PersonModel). If I add another Person (via CURL or in a seperate window), PersonCollection correctly fires a "created" event. However, the PersonCollection itself is not being updated with the new Person. Only when I manually call personCollection.add(person) inside the event callback does the PersonCollection reflect the update.

Is this by design? Could I be doing something wrong?

            var personCollection = new PersonCollection();

            personCollection.on('created', function (model) {
                // personCollection does not contain the new model yet :(
                // I have to add it manually
                personCollection.add(model);
            });

            React.render(new (React.createFactory(HomeComponent))({
                collection: personCollection
            }), mountPoint);

            personCollection.fetch({
                sync: ['socket', 'ajax', 'subscribe', 'set']
            });
ianhaggerty commented 9 years ago

Hello

Thanks for your interest and comments!

This is a design decision first and foremost. For the 0.1 version I wanted to keep things as low level as possible, and then (very, very carefully) build on top of them with sub classed collections & models. As such, there is no default response to the created event - it doesn't add the new person, you have to do that yourself (this would seriously conflict with any where criteria you've specified otherwise). The same is true of the updated event, it is up to you to update your client side model with the relevant information.

If you want to populate your created model you'll need to send another request to the server - there is no population through socket events, and there is currently no way of configuring that server side.

Hope that helps

hardchor commented 9 years ago

I understand that it's probably not the easiest thing in the world to get right! Also, converting where criteria to backbone collection filters might be tricky (would have to read through the backbone documentation).

Are you going to stick to coffeescript moving forward? I wouldn't mind contributing, but don't "speak" coffeescript.

ianhaggerty commented 9 years ago

I didn't consider developer involvement when I started this out in coffee-script, but in retrospect, I don't think it would have ever got finished if I hadn't. I would love for a few more people to get involved with the development, but I don't think I'll be rewriting this in JavaScript. It simply plays too nice with promises & backbone class system to justify a rewrite & the readability is a huge boon.

I would go so far as to recommend a combination of coffee-script & bluebird promises, it allows you to do things like this:

master = new Model( name: "master" )
master.save().then ->
  added = new Model( name: "added" )
  added.save().then ->
    master.set('test', added).save()
  .then ->
    master.populate("test").fetch()
  .then ->
    expect(master.get("test")?.name).toEqual("added")
    addedTo = new Model( name: "addedTo", tests: [master] )
    addedTo.save().then ->
      master.populate().fetch()
    .then ->
      expect(master.get("test")).toEqual(addedTo.id)
      master.populate("test").fetch()
    .then ->
      expect(master.get("test").id).toEqual(addedTo.id)
      addedTo = master.get("test", true)
      addedTo.populate("tests").fetch()
    .then ->
      expect(addedTo.get("tests")?[0]?.name).toEqual("master")
      addedTo.populate("tests").set("tests", []).save()
    .then ->
      expect(addedTo.get("tests")?.length).toEqual(0)
      master.populate(false).fetch()
    .then ->
      expect(master.get("test")).not.toBeTruthy()
      added.populate("tests").set("tests", []).save()
    .then ->
      expect(added.get("tests")?.length).toEqual(0)
      master.populate(false).fetch()
    .then ->
      expect(master.get("test")).not.toBeTruthy()
      done()

Anyhow, keep the issues coming! The more people I know are using this plugin and are interested in seeing it flourish, the more inspired I'll be to work on it.

hardchor commented 9 years ago

Hi @oscarhaggerty

Just had time to pick this up again. That coffeescript example you posted would be largely the same in JS (unless I'm mistaken).

Anyway, what's your thoughts on converting where criteria to Backbone collection filters and automatically update collections based on them? Feasible? Too difficult to get right?

ianhaggerty commented 9 years ago

It would bloat the front end somewhat.

You'd have to parse the Waterline where criteria on the client. The team behind Sails have spliced this out as a separate repository already. So it is very do-able.

You couldn't filter by associated attributes. A lot of created events would be sent unnecessarily. It seems like a problem best solved by the server side, with an extended client side API. But I am not about to edit the sails core...

In light of that, the only feasible way of doing it would be to migrate waterline-criteria into Backbone.Sails. Once that is done, it wouldn't take long to produce a new collection class which maintains itself in sync with the server-side state.

Heck, I'll start a v0.2 branch right now and get to it. It won't take long :)

hardchor commented 9 years ago

Haha now that's ambition!

On Thu, 29 Jan 2015 21:01 Oscar Haggerty notifications@github.com wrote:

It would bloat the front end somewhat.

You'd have to parse the Waterline where criteria on the client. The team behind Sails have spliced this out as a separate repository https://github.com/balderdashy/waterline-criteria already. So it is very do-able.

You couldn't filter by associated attributes. A lot of created events would be sent unnecessarily. It seems like a problem best solved by the server side, with an extended client side API. But I am not about to edit the sails core...

In light of that, the only feasible way of doing it would be to migrate waterline-criteria https://github.com/balderdashy/waterline-criteria into Backbone.Sails. Once that is done, it wouldn't take long to produce a new collection class which maintains itself in sync with the server-side state.

Heck, I'll start a v0.2 branch right now and get to it. It won't take long :)

— Reply to this email directly or view it on GitHub https://github.com/oscarhaggerty/Backbone.Sails/issues/4#issuecomment-72103278 .

ianhaggerty commented 9 years ago

Just created the branch. You can find the developmental release here.

The API I came up was this:

coll.query({
  where: { name: { startsWith: "I" } },
  sort: "name ASC"
}); // example filter criteria

// fetch over socket (or delegate to ajax first)
coll.fetch({sync:'socket'}).then(function(){

  // and now for the magic...
  coll.synchronize();
  // this sets up relevant listeners to maintain the state of the collection
  // with that the server would otherwise return via a fetch()
  // populated attributes not supported

  // not interested anymore?
  coll.desynchronize();
  // this'll tear down the socket event listeners
}

This also works for an individual model. Untested as of yet (bar some simple tests with chrome console). You will need to download the developmental release on branch 'v0.2'. Waterline-criteria also needs to be included before.

hardchor commented 9 years ago

Brilliant stuff! I'll give it a go this evening.