Attribute rework

[danwrong]

This patch introduces this.attributes (which we alias to this.defaultAttrs) which improves attributes interaction with mixins and introduces some extra assertions to help developers.

It's all backwards compatible apart from one big change (which I think we need to be cautious about before merging):

If you pass any attributes into attachTo that the component has not declared then they are ignored. Example:

var F = flight.component(function() {
  this.defaultAttrs({
    thing: 1
  });
});

F.attachTo(elem, { thang: 1, thing: 10 });

// Old attrs
// { thang: 1, thing: 10 }

// After this change
// { thing: 10 }

I think this will be a breaking change for Twitter's code base. In might well be for others. Thoughts?

UPDATE: I've completely separated out the old a new behaviour and new behavior. If you define a component using this.defaultAttrs then you get old behaviour, if you use this.attributes you get new behaviour. Now a non-breaking change.

Also, we might want to go to a read only attribute model while we are changing this. Two approaches here:

1. Develop time check using getters so API uses this.attr as is does now.
2. Use a method:

this.attribute('thing') //=> returns value
this.$attribute('thing') //=> returns jQueried version of that value

This would allow us to do some interesting things because we could around this function in components to add behavior such as logging.

[sayrer]

Should this be for-in? I'm not sure. We discussed in person.

[necolas]

This could be a good time to address #141 too.

[necolas]

Is it worth aliasing to this.defaultAttrs if there's a breaking change?

[danwrong]

@necolas: Indeed, originally had it so that this.attributes was an alternate system that worked alongside this.attributes. Think I might go back to that.

[tgvashworth]

What's the thinking on this now, a few months later?

This might break TweetDeck, but not badly. Afaik, we're only passing an object to attachTo in a few places and from a quick scan of the code, the attributes are defined in most places.

One thing I'd like to add to this discussion: a misconception I had when starting with Flight was that the component function was run once per instance at attach time. That meant I did things like:

this.defaultAttrs({
  rifftumbles: []
});

this.somethingGood = function () {
  this.attr.rifftumbles.push({
    beep: 'boop'
  });
};

Of course, then the rifftumbles are shared between instances of the component.

My question then is: should instance-specific state go in instance.attr? If so, is there a possible syntax that avoids this kind of thing?

This seems to verbose, not to mention the redundant static data it would produce.

this.attributes(function () {
  return {
    boopSelector: '.boop',
    rifftumbles: []
  };
});

[necolas]

Use single spaces after .

[angus-c]

I think we should only assign an identity if there isn't already one - initialize can be called multiple times

this.identity || (this.identity = componentId++)

[danwrong]

That is a good plan, its not part of my patch though, I just moved it around. Maybe address as a separate commit?

[angus-c]

It was done in the pre-patch code :)