length-property

[gnimmelf]

Hi,

I have a rest-collection that I want to wait for. It is set on the view as such:

View.extend({
  initialize: function() {
    this.eventCollection = new HSEventCollection({
      url: url,
    });

   console.log(this.eventCollection.length)

   // this.eventCollection.fetch(); // <-- commented out to see what gets rendered
  }
})

Then, I want to wait for it to load before rendering in a sub-view:

subviews: {
  list: {
    container: '.item-container',
    waitFor: 'eventCollection.length',
    prepareView: function (el) {
      debug('prepareView', this.el, this.eventCollection)
      return new CollectionRenderer({
        el: el,
        collection: this.eventCollection,
        view: HSEventView
      });
    }
  },
}

The problem is that eventCollection.length is 1 before the fetch. It seems to contain an empty model.

Changing the log-statement to JSON.stringify(this.eventCollection.toJSON()) gives: "[{}].

Any ideas on what's going on / what I'm (again) missing?

Cheers!

Edit: Just realized I cannot waitFor ''eventCollection.length'' as it is does not fire a change event in and by itself. Still the length issue...

[gnimmelf]

I might be very off here, but

This is from the ampersand-collection@1.4.5 dependency in ampersand-rest-collection@4.0.0:

function Collection(models, options) {
    options || (options = {});
    if (options.model) this.model = options.model;
    if (options.comparator) this.comparator = options.comparator;
    if (options.parent) this.parent = options.parent;
    if (!this.mainIndex) {
        var idAttribute = this.model && this.model.prototype && this.model.prototype.idAttribute;
        this.mainIndex = idAttribute || 'id';
    }
    this._reset();
    this.initialize.apply(this, arguments);

    console.info(this.length, models, options)
    // => "0, Object {url: "/REST/event/orgs/1/events"}, Object {}"

    if (models) this.reset(models, assign({silent: true}, options));

    console.info(this.length, models, options)
    // => "1, Object {url: "/REST/event/orgs/1/events"}, Object {}
}

The models-param IS the options from

new HSEventCollection({
      url: url,
    });

and the options is an empty object!?

I have no idea how this actually works, but this does not look right to me.

Edit: There seems to be other stuff in the collection-constructor as well. initialize should only accept an options argument, but it gets called with the constructor-arguments applied, so the models argument ends up being passed as the first argument. Should maybe be this.initialize.call(this, options, models) instead of this.initialize.apply(this, arguments)

[lukekarrys]

When instantiating a new collection, the first argument is the models for the collection. url on the other hand is a property that goes in the collection.extend method.

var HSEventCollection = AmpersandCollection.extend({
  url: url
});

this.eventCollection = new HSEventCollection();
// or if you wanted to instantiate with initial models
this.eventCollection = new HSEventCollection([model1, model2, ...]);

[gnimmelf]

Hm, I see.

The docs say "new AmpersandCollection([models], [options])", so I assumed both to be independently optional (which they are not), and that would allow me to pass only an options-object to the "collection.initialize" function.

I'll have another look at it tomorrow.

Thanks.

[gnimmelf]

Ok, this is turning into another issue, but I'll just keep going a bit more before I close this, and open an issue on ampersand-model.

So, I read the collection docs as clearly saying that the constructor takes two optinal arguments

constructor/initialize new AmpersandCollection([models], [options])

-meaning that

var options = {
      url: url, // Or whatever...
}
var collection = new AmpersandCollection(options);

is perfectly legal. However, the code require the first argument to be present if you are passing the second argument, so the docs should in that case be:

constructor/initialize new AmpersandCollection([models, [options]])

I have never used Backbone, so if this is idiomatic there, I will just adopt to that convention. Otherwise, it I'd think this would make more sense:

function Collection(models, options) {

    // HERE!
    if (isObject(models) && options === undefined) {
        options = models;
        models = null;
    }

    options || (options = {});
    if (options.model) this.model = options.model;
    if (options.comparator) this.comparator = options.comparator;
    if (options.parent) this.parent = options.parent;
    if (!this.mainIndex) {
        var idAttribute = this.model && this.model.prototype && this.model.prototype.idAttribute;
        this.mainIndex = idAttribute || 'id';
    }
    this._reset();

    // AND HERE
    this.initialize.call(this, options, models);

    if (models) this.reset(models, assign({silent: true}, options));
}

-as all other initialize-methods takes the options as first parameter.

Thanks again.

[lukekarrys]

I believe this part was copied over from Backbone. From the Backbone collection constructor docs:

var spaces = new Backbone.Collection([], {
  model: Space
});

And passing options as the first parameter creates a collection with a length of 1. I'm not sure how idiomatic it is, but I know as a Backbone user for a few years before Ampersand the syntax of new Collection({url: url}) would be confusing to me :smile:

I do agree that the docs are confusing. Would you be up for a PR to change that part?

[gnimmelf]

Yes, I'll do that.