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

Common Helpers #3

Closed
aldeed opened this issue Jan 17, 2014 · 20 comments
Closed

Common Helpers #3

aldeed opened this issue Jan 17, 2014 · 20 comments

Comments

@aldeed
Copy link

aldeed commented Jan 17, 2014

The method used in this package is great. I'm thinking of deprecating virtualFields support in collection2 and telling everyone to use this package instead. It's better because the methods are called only as necessary.

Anyway, I suggest an option to define common helpers more easily. Like this:

One = new Meteor.Collection('one');
Two = new Meteor.Collection('two');
Three = new Meteor.Collection('three');
Four = new Meteor.Collection('four');

CollectionHelpers.addHelpers({
  creator: function() {
    return Meteor.users.findOne(this.createdBy);
  }
}, [One, Two, Three, Four]);

In apps with many collections that share many common properties, this could simplify the code a lot.

@serkandurusoy
Copy link

This would be a great pattern and has many use cases in common data access features such as the example given.

@dburles
Copy link
Owner

dburles commented Jan 17, 2014

Hey, I was actually pondering this myself, I think this is a good idea. I'll have a play with some ideas over the weekend :)

@dburles
Copy link
Owner

dburles commented Jan 17, 2014

How does this look?

// We can define Meteor.Collection.helpers as many times as we like
Meteor.Collection.helpers({
  dog: 'woof'
}, [Books, Authors]);

Meteor.Collection.helpers({
  cat: 'meow'
}, [Books]);

Books.findOne().dog; // "woof"
Authors.findOne().dog; // "woof"
Books.findOne().cat; // "meow"
Authors.findOne().cat; // undefined

@dburles
Copy link
Owner

dburles commented Jan 17, 2014

Actually having thought about it some more, I don't think it's wise to attach the helpers method to Meteor.Collection as strictly speaking it's a constructor. Perhaps I'm being too strict, will ponder.

dburles added a commit that referenced this issue Jan 17, 2014
@dburles
Copy link
Owner

dburles commented Jan 17, 2014

I'm still not 100% sure on this approach but here it is, please give it a try :)

@aldeed
Copy link
Author

aldeed commented Jan 17, 2014

Awesome, thanks. I commented on the commit, but otherwise looks perfect. I often attach methods to constructor functions, using them as objects as well. I don't know if it's kosher, but it's fine with me. :)

I'll be able to test it out tomorrow.

@serkandurusoy
Copy link

Definitely +1

@dburles
Copy link
Owner

dburles commented Jan 17, 2014

Cool good catch, not sure how I didn't notice that, aw well. I'll fix it up in the morning :)

@dburles
Copy link
Owner

dburles commented Jan 17, 2014

@aldeed
Copy link
Author

aldeed commented Jan 18, 2014

Perfect! Thanks for the quick turnaround. I can test tomorrow, but the code looks good to me.

@serkandurusoy
Copy link

@dburles when do you plan on releasing this on atmosphere?

@dburles
Copy link
Owner

dburles commented Jan 19, 2014

Hey @serkandurusoy not 100% settled on the api yet however you can try it through meteorite by editing your smart.json and adding:

    "collection-helpers": {
      "git": "https://github.com/dburles/meteor-collection-helpers.git",
      "branch": "common-helpers"
    }

@serkandurusoy
Copy link

I keep dismissing how coupled with git atmoshpere is, thanks :)

@aldeed
Copy link
Author

aldeed commented Jan 20, 2014

This works fine in my tests. No preference on the API, for me.

@dburles
Copy link
Owner

dburles commented Jan 23, 2014

Hey guys, been thinking about this over the past few days. I think that it's easy enough to achieve this without needlessly adding further complexity to the package in the form of another api.

For example we can already achieve much the same thing by creating mixins, like:

// this might live somewhere like collections/lib/mixins.js
hasTimeHelpers = function(collection) {
  collection.helpers({
    formattedCreatedAt: function() {
      return moment(this.createdAt).format('MMMM Do YYYY, h:mm a');
    },
    formattedUpdatedAt: function() {
      return moment(this.updatedAt).format('MMMM Do YYYY, h:mm a');
    }
  });
};

// collections/authors.js
Authors = new Meteor.Collection('authors');

hasTimeHelpers(Authors);

Authors.helpers({
  fullName: function() {
    return this.firstName + ' ' + this.lastName;
  },
  books: function() {
    return Books.find({ authorId: this._id });
  }
});

@serkandurusoy
Copy link

From a personal/developer perspective, either way of declaring the common helpers looks just fine to me. Somehow, your first proposal felt more natural, though.

@aldeed
Copy link
Author

aldeed commented Jan 23, 2014

I was doing something like your mixin example when I decided to suggest this feature. For some reason, it felt like having a built-in way would be better. However, you're correct that there doesn't seem to be much difference.

I might suggest mentioning this coding pattern in the readme, probably just pasting the example you posted here. It's probably fairly obvious, but it's always nice to see that a package author has thought of a certain scenario and has best-practice recommendations for it.

Otherwise, I'm okay with closing this.

@dburles
Copy link
Owner

dburles commented Mar 12, 2014

Hey, thought i'd post this, here's another way to apply helpers across many collections.
This method is mush closer to the proposed API than the mixin method above.

// Time helpers
_.each([Books, Authors], function(collection) {
  collection.helpers({
    formattedCreatedAt: function() {
      return moment(this.createdAt).format('MMMM Do YYYY, h:mm a');
    },
    formattedUpdatedAt: function() {
      return moment(this.updatedAt).format('MMMM Do YYYY, h:mm a');
    }
  });
});

Not sure why I didn't think about it at the time

@dburles dburles closed this as completed Mar 30, 2014
@queso
Copy link

queso commented Sep 13, 2014

Will this 'stack' so if I define a collection helper like this and then go into each collection and define more helpers, will they all get applied?

@aldeed
Copy link
Author

aldeed commented Sep 13, 2014

@queso, yes it will extend. So they'll get added or if they have the same name, the one defined later will overwrite the earlier one.

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

4 participants