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

Support for run-level partialsDir #118

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

nealshail
Copy link

I'm developing a site with a 'themeable' single route (or group of route). This is so a we can customise a given page easily using partials as hooks. For the majority of the other pages (the administration backend) no additional partials are required. So I've added the ability to pass an additional partialsDir while calling res.render()

res.render('viewer/index', {
        layout: false,
        partialsDir: req.theme.partialsDir,
    });

this overrides any default partials with the themes partials. I've also made it possible to send in array of partials..

res.render('viewer/index', {
        layout: false,
        partialsDir: [req.default.partialsDir,req.theme.partialsDir]
    });

@ericf
Copy link
Owner

ericf commented Mar 25, 2015

What does the code look like that you have to use today without this feature?

[req.theme.partialsDir, req.default.partialsDir]

I think this should be reversed: defaults coming before the theme's partials.

@nealshail
Copy link
Author

yes you are correct, it should be the other way around to override. I miswrote the pseudocode and have updated the comment now.

@ericf
Copy link
Owner

ericf commented Mar 25, 2015

I think it might be confusing to have two partials related render-time options:

  • partials: Collection {templateName: templateFn}
  • partialsDir: String dirname or config object

Which one takes precedence if both are provided?

I'm not sure how popular these two features would be, but we could change what you're proposing here to be expressible using the public API without adding a new render-time option:

res.render('viewer/index', {
  layout: false,
  partials: exphbs.getPartials({
    cache: req.app.get('view cache'),
    dirs: [
      req.default.partialsDir,
      req.theme.partialsDir
    ]
  })
});

To implement this, we could add a option.dirs to the getPartials() method that follows the same signature as partialsDirs config option, and it would default to this.partialsDir.

@nealshail
Copy link
Author

yes this could work, though it adds an extra dependancy to exphbs which breaks the abstraction of the generic templating api that req.render() provides. At the moment the exphbs object is created during express configuration and startup, but my viewController code is way over the other side of my project, so i'd have to store it globally somewhere to be able to access it. It just feels like I'm adding a dependency to something 'under the hood'.

A natural partials Hierarchy does seem logical as you suggest.

  • partials: Collection {templateName: templateFn}
  • partialsDir: String dirname or config object

@ericf
Copy link
Owner

ericf commented Mar 26, 2015

I agree that breaking the abstraction is not great. Let me think about this some more… and let's see if we can come up with something that's easier for people to reason about and understand how the concepts of partials and partialsDir would interact.

@nealshail
Copy link
Author

I was thinking about the partials parameter.. In it's current form it also has a dependancy on exphbs for it to be useful, am i right, it will have the same problem?

When i first saw the feature i thought it would uniquely work with files and intuitively take the form partials: { mypartial:"path/to/the/partial/file.hbs"} rather than the actual promise of the resolved template using the exphbs instance interface.

If one wanted to add a single partial in the form of a small generated string (like in the advanced example) wouldn't it most likely be better to pass it in as a regular handlebars {{parameter}}, and just construct all the elements on the fly before sending it in, at this stage one would be in the javascript code anyway... Forgive me but i have difficulty seeing the use case for the single partial parameter which isn't a file. but maybe I'm just biased..?

@nealshail
Copy link
Author

for my use case I could also add the partials manually (as i know which partials are included in the specific view I'm loading).. if i could have an interface to pass in many partials at a time, i.e.

partials: [{ mypartial:"path/to/the/partial/file.hbs"}, {myotherpartial:"path/to/the/other/partial/file.hbs"}]

this may work for me in place of the partialsDir solution.

@nealshail
Copy link
Author

Hi Ericf, Sorry it's been a while, this project has been on ice for a while.. Any more ideas on adding this feature for me?

Use case: In our application, a single endpoint (not site wide) can be configured with different themes, as part of the theme one has the ability to override specific partials. Added support to add additional partials at run-level
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.

2 participants