longshotlabs / meteor-template-extension

A Meteor package: Replace already defined templates, inherit helpers and events from other templates
https://atmospherejs.com/aldeed/template-extension
MIT License
220 stars 20 forks source link

Cannot add helpers to inheriting template #30

Closed SachaG closed 8 years ago

SachaG commented 9 years ago

In the documentation's example:

Template.foo.bar = function () {
  return "TEST";
};

Template.foo.events({
  'click button': function (event, template) {
    console.log("foo button clicked");
  }
});

Template.foo2.replaces("foo");

The {{bar}} helper that was available on foo becomes available on foo2 as well.

But if I want to add a new helper to foo2, should I add it on foo or foo2? Or is it the same thing?

So far from my testing it seems like helpers only work if they're added to foo, which seems a little counter-intuitive since I want to add them inside foo2?

spencern commented 9 years ago

I'm having this same issue as well and additionally using Template.foo2.rendered doesn't ever seem to trigger and I must put calls inside of Template.foo.rendered instead (they work there). Agreed with @SachaG that it seems counter-intuitive.

For context, I'm overriding Templates that exist in one package from another package.

aldeed commented 9 years ago

Yeah, I'm not sure how easy it would be to fix this due to that way this is hacking things. It's an example of an area where the implementation of the API could be much nicer if this becomes part of the core packages.

For the life cycle callbacks, you should use hooks if overriding. That may fix the issues @spencern is having with rendered.

I still need to test this more thoroughly with the new onRendered, etc. syntax. We may need some package updates to work better with latest Meteor.

SachaG commented 9 years ago

What can we do to help make this become part of the core?

aldeed commented 9 years ago

It seems like MDG is ok with a PR for adding some of it. To do override/inherit correctly might be somewhat complex, though. Right now this package essentially just copies the lists/functions from one template to the other, which makes it difficult to properly add to the list after setting up the relationship. If we can refactor that into a better inheritance logic, then it should be suitable for PR.

The first step might be to add tiny tests (some of which will fail) for all the cases of adding/removing hooks, helpers, and events after doing replace/inherit.

aldeed commented 8 years ago

replaces only replaces the render function. All other aspects like helpers, events, and callbacks still are on the replaced template. I've updated the readme to clarify this.

We could think about a more advanced function that would allow you to replace and also extend.