Closed joanniclaborde closed 9 years ago
@pushred I moved the changes from #118 into this PR, it's ready to be merged into master, unless you'd like to try it out first.
Thanks, I'll give it a quick go tomorrow. If the tests pass, it should be good though!
Well the basics work, I rendered a template from Josiah's subscribe module into a site. That was pretty cool! But wondering about partials & helpers. Anyway we could register those at least? Autodiscovery of partials would be nice, with the same treatment they get in Solidus. In my case I have:
/src/template.hbs
/src/test.hbs
... and in template.hbs
I have:
{{> test }}
which results in:
Error: The partial test could not be found
at new Error (<anonymous>)
at Error.Handlebars.Exception (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/utils.js:10:41)
at Object.Handlebars.VM.invokePartial (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/runtime.js:88:13)
at Object.eval (eval at <anonymous> (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/compiler/compiler.js:579:23), <anonymous>:16:17)
at /Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/runtime.js:38:33
at /Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/compiler/compiler.js:1294:21
at Object.Handlebars.VM.invokePartial (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/runtime.js:90:14)
at Object.eval (eval at <anonymous> (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/compiler/compiler.js:579:23), <anonymous>:8:17)
at /Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/runtime.js:38:33
at /Users/eric/dev/sites/project/node_modules/solidus/node_modules/handlebars/lib/handlebars/compiler/compiler.js:1294:21
at ExpressHandlebars._renderTemplate (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/express-handlebars/lib/express-handlebars.js:310:12)
at ExpressHandlebars.<anonymous> (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/express-handlebars/lib/express-handlebars.js:184:21)
at /Users/eric/dev/sites/project/node_modules/solidus/node_modules/express-handlebars/node_modules/promise/lib/core.js:33:15
at flush (/Users/eric/dev/sites/project/node_modules/solidus/node_modules/express-handlebars/node_modules/promise/node_modules/asap/asap.js:27:13)
at process._tickCallback (node.js:419:13)
Helpers are trickier. One way could be requiring any dependencies into helpers.js
, that'd just entail a bit of wiring. Maybe modules can have a helpers.js
of their own that define any custom helpers and require any dependencies of their own?
Missing partials: I'll see what I can do, I didn't think about that.
Missing helpers: If we follow the pattern established in https://github.com/solidusjs/solidus/pull/116 and https://github.com/solidusjs/solidus-client/pull/1, each module would be responsible of exporting its helpers (accessible as require('some-module).template_options.helpers
). Solidus could then automatically include those helpers, instead of trying to require('some-module/helpers')
.
Speaking of which, the modules will also export the partials they need, maybe we could use that info instead of this whole node_modules
business in this PR... I'll need to think about this a bit more, and do some tests...
I don't see how we can cleanly fix partials called from a package's partial. We can either:
{{> some-module/test}}
instead of {{> test}}
in your example). This will make the package difficult to use standalone, the partials will need to be registered with those names too.node_modules
as a partials dir, add each dependency path one by one, and hope there are no partials names conflicts between the site and the packages.I think solution 1 is the way to go. We'll only support partials made for Solidus. We actually don't even support random partials right now: if I was to render a 3rd-party template that uses a partial, package would need to already be using some kind of file system paths for the partials names, that's not the standard Handlebars way to use partials.
Namespacing the partials is reasonable, the sort of use case I'm thinking for partials is a pattern we commonly follow in sites where we markup a content item (i.e. a tweet, photo, blog post) and include those partials into different index templates. For instance a carousel for featuring content, a paginated archive, or even perhaps reusing the same template to display the content. I might want to require such content markup from a module while doing something more custom for the index.
Whatever we do for helpers we'll have to ensure that it's easy to keep their availability consistent between the browser and server. I fixed one issue today where a developer had forgotten to mix them into their clientside code failing clientside rendering on the entire page (while still working serverside, minus session data).
It's the view's responsibility to export the helpers it needs to render. Then the dev can do something like this in his site's helpers.js:
var _ = require('underscore');
var view1 = require('some-module/some-view1');
var view2 = require('some-module/some-view2');
module.exports = _.extend({},
view1.template_options.helpers,
view2.template_options.helpers,
{
myOwnHelper: function() {
// ...
}
}
);
This will make the external helpers available when the templates are rendered. I'm afraid there's nothing we can do about name conflicts, since Handlebars expects a single object containing all helpers required to render the template and partials.
And since the helpers are part of the view exported by an external package, they are always available when rendering client-side too.
Note: this is merged into https://github.com/solidusjs/solidus/tree/1.0.0
Warning: this was rebased over master, delete your local branches!
This PR adds the site's
node_modules
folder as a partials dir in express-handlebars. All modules installed by the site can then be used for partials:Note that those views will always be cached, even in dev.
Fixes #104. Fixes #123.