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

Add support for render-level partials #107

Merged
merged 1 commit into from
Feb 17, 2015
Merged

Add support for render-level partials #107

merged 1 commit into from
Feb 17, 2015

Conversation

ericf
Copy link
Owner

@ericf ericf commented Feb 17, 2015

The renderView() method now accepts render-level partials that are merged with the instance-level and global Handlebars partials. The option value is specified as object with the shape: {partialName: fn} or a Promise of such an object.

Fixes #82

The `renderView()` method now accepts render-level `partials` that are
merged with the instance-level and global Handlebars partials. The
option value is specified as object with the shape: `{partialName: fn}`
or a Promise of such an object.

Fixes #82

var helpers = utils.extend({},
this.handlebars.helpers, this.helpers, options.helpers);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the merge of this.handlebars.helpers since Handlebars will do this merge automatically at render time.

@caridy
Copy link

caridy commented Feb 17, 2015

+1

ericf added a commit that referenced this pull request Feb 17, 2015
Add support for render-level partials
@ericf ericf merged commit 808c1c7 into master Feb 17, 2015
@ericf ericf deleted the partials-option branch February 17, 2015 22:28
@ericf
Copy link
Owner Author

ericf commented Feb 17, 2015

Released in v1.2.0.

@nealshail
Copy link

Thanks for this feature, though I'm unsure of the exact syntax to use..

'specified as object with the shape: {partialName: fn}

I've used {'mypartial':'path/to/my/partial/mypartial.hbs'} but this is only adding 'path/to/my/partial/template.hbs' as text in the place of the partial. How should we resolve this to an object or a promise of an object? Do you have an example?

@caridy
Copy link

caridy commented Mar 25, 2015

It is an promise that resolves to a map, you can use getTemplate() method off the instance to fetch those partials. We can probably improve that by providing a easy way to define each partial as a promise in the map, we just need to convince @ericf jejejeje.

@ericf
Copy link
Owner Author

ericf commented Mar 25, 2015

@nealshail It's a hash of "partialName"s to compiled Handlebars template functions. Or it can be a promise whose value resolves to an object in that shape:

{
  foo: function () { /* compiled Handlebars template */ },
  bar: function () { /* compiled Handlebars template */ }
}

@ericf
Copy link
Owner Author

ericf commented Mar 25, 2015

We can probably improve that by providing a easy way to define each partial as a promise in the map

@caridy Not sure I follow…what are you thinking?

@caridy
Copy link

caridy commented Mar 25, 2015

@ericf we have a similar situation with MBR. It is just messy to have to create a bunch of promises (one per partial) then aggregate them into a map after calling Promise.all(). Maybe we can abstract that part of the operation into exp-hbs directly, so they only need to create a map, something like:

{
    foo: obj.getTemplate('/path/to/foo.hbs'),
    bar: obj.getTemplate('/path/to/bar.hbs')
}

@ericf
Copy link
Owner Author

ericf commented Mar 25, 2015

@caridy would that be covered by what I wrote here #118 (comment)?

@caridy
Copy link

caridy commented Mar 25, 2015

@ericf that's a no go since dirs means nothing when you have contextualization.

@ericf
Copy link
Owner Author

ericf commented Mar 25, 2015

Well the API currently provides what's needed to implement what you want, it sounds like the app needs an abstraction over this to create an more DLS-y API. I don't care if people have to write a loop/forEach to accomplish the complex use they have…

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

Successfully merging this pull request may close these issues.

Partials as an Option
3 participants