cmather / blaze-layout

A Meteor Blaze powered layout component for dynamic rendering.
Other
49 stars 8 forks source link

Remove Component.lookup and Component.lookupTemplate overrides #22

Open cmather opened 10 years ago

cmather commented 10 years ago

cc @avital, @estark37, @tmeasday

In order to make "yield" and "contentFor" work with blaze in the last release of blaze-layout/iron-router, we overrode Component.lookup and Component.lookupTemplate. This is now breaking with additions that have been made to these functions in Core (related to UI._templateInstance()) in release/0.8.1-rc2. See: https://github.com/EventedMind/iron-router/issues/691#issuecomment-46125097.

We should remove these overrides provided we can meet or change our original use cases. Those use cases are:

  1. When {{> yield}} is called, find the parent Layout component and call the yield method defined on that component.
  2. When {{> contentFor}} is called, find the parent Layout component and call the contentFor method defined on that component.
  3. When {{yield}} is called provide a helpful error message like: "Sorry, would you mind using {{> yield}} instead of {{yield}}?"

It's not immediately clear how we can tell Spacebars to lookup a rendered hierarchy of components for a method defined further up in the chain. But I recall some designs for this in Meteor's Blaze v2 hackpad. I need to verify that something exists. Alternatively, we can add the UI._templateInstance implementation logic to our override. But I'm hoping we can remove this hack.

tmeasday commented 10 years ago

@cmather - I think we are still waiting on Enable dynamically scoped helpers on components, with restrictions -- but in the meantime perhaps we can do something like this:

UI.registerHelper('yield', function() {
  var template = UI._templateInstance();
  var component = template.__component__;

  // find parent Layout component
  while (component && component.kind !== 'Layout')
    component = component.parent;

  if (! component) throw "NOT IN A LAYOUT";

  return component.yield.call(this, arguments);
});

Haven't tried that, just speculating...

cmather commented 10 years ago

Yeah that looks promising.

On Jun 15, 2014, at 4:55 PM, Tom Coleman notifications@github.com wrote:

@cmather - I think we are still waiting on Enable dynamically scoped helpers on components, with restrictions -- but in the meantime perhaps we can do something like this:

UI.registerHelper('yield', function() { var template = UI._templateInstance(); var component = template.component;

// find parent Layout component while (component && component.kind !== 'Layout') component = component.parent;

if (! component) throw "NOT IN A LAYOUT";

return component.yield.call(this, arguments); }); Haven't tried that, just speculating...

— Reply to this email directly or view it on GitHub.

davidworkman9 commented 10 years ago

I was able to remove the override of UI.Component.lookup and confirm that UI._parentData(n) and UI._templateInstance() both work now. The rest of my app seems to work fine too, I'm not quite sure I fully grok what the override is trying to do, but my app doesn't seem to need it.

cmather commented 10 years ago

This is all fixed up with a new layout package: https://github.com/eventedmind/iron-layout

IronRouter integration is on a feature branch: feature/iron-layout. After some app testing we'll do a patch release.