templates should have access to the raw "contents" of other files

metalsmith / layouts

A metalsmith plugin for layouts

MIT License

116 stars 49 forks source link

templates should have access to the raw "contents" of other files #175

Closed webketje closed 3 years ago

webketje commented 3 years ago

Access to the text content without HTML layout (raw contents) is a common need for list pages. To that effect, metalsmith-layouts should only re-assign file values in the files object with compiled content after all compilations are done. To illustrate, see the example below:

Given the following metalsmith build chain:

metalsmith
  .use(collections({
    articles: 'src/articles/*.md'
  })
  .use(layouts());

... an excerpt of the Handlebars template article-overview.hbs:

<ul>
{{#each articles }}
  <li>
    <h2>{{ title }}</h2>
    <p>{{{ contents }}}</p>
  </li>
{{/each}}
</ul>

... the Handlebars template article.hbs:

<!DOCTYPE html>
<html>
  <body>{{{ contents }}}</body>
</html>

...and the articles having a format like:

---
title: Article 1
layout: article.hbs
---
Hello world!

the {{{ contents }}} rendered in article-overview.hbs should not be wrapped in <body> tags

ismay commented 3 years ago

Hey, yeah listing items from a collection is a common usecase. However, it's not as necessary to have access to the contents of the file before it was transformed. I'll explain:

When iterating through a list of articles you're probably not rendering the entire contents of the article in that list. So a solution I'd advocate is adding a description key to the frontmatter (for example), with a small excerpt of the article. Which you can then use for rendering in the list in the article overview (and the meta description of the article page for example):

<ul>
{{#each articles }}
  <li>
    <h2>{{ title }}</h2>
    <p>{{ description }}</p>
  </li>
{{/each}}
</ul>

Or you could use https://github.com/segmentio/metalsmith-excerpts as well. Or even duplicate file contents to another property than contents on the metalsmith file object before layouts transforms the contents, if you really need the entire contents.

So I think your usecase is valid, but I feel it's very solveable without making changes to layouts. In fact, I'd say it's better to not solve it in layouts because it keeps layouts simpler and more predictable (both for the user and the maintainers).

ismay commented 3 years ago

I'll close this, but feel free to comment if anything's unclear.

webketje commented 3 years ago

I use your solution as well. I created this issue as follow-up to a support thread on gitter where I saw that this was clearly the expected behavior for a metalsmith newcomer. Your comment made me consider the need for this feature thoroughly, and I came to a different conclusion.

While it is not strictly necessary, the change is definitely useful in the light of making metalsmith more accessible (especially for new users):

Expected behavior for newcomers. Making the change doesn't remove any functionality, but adds to it. Now you cannot use the contents directly (unless you want wildly invalid HTML with <!DOCTYPE html> sprinkled across the page), then you would have that choice.
Other solutions alluded to (excerpts, extra property) put the burden on the developer, require (slightly) more advanced knowledge of metalsmith and an extra frontmatter property, custom plugin or NPM dependency.
It allows to use the power of established templating languages instead of reinventing a solution: in twig templating for example, you could do {{ contents|excerpt(100) }}. Currently, this will yield unsatisfactory results.
It makes the behavior of rendering collection {{{ contents }}} of other files predictable. Currently whether you will get prerendered contents depends purely on file system order (alphabetical)
The change to make doesn't look that complex: it's simply a matter of assigning to a temp files object, and only replacing the contents property after all files are rendered.

ismay commented 3 years ago

The thing is, if you want to understand the metalsmith ecosystem, the unix philosophy is a good analogy: simple, composable, single responsibility tools. So keeping things simple is a goal in and of itself.

Now the intention of making things simple for newcomers is good. But I don't want to smooth over wrong assumptions made by newcomers. I'd rather educate them on the mistake with good examples and documentation.

Plus most (if not all) metalsmith plugins operate directly on the files object. So we'd be going against the established pattern, which is also not a good idea in my opinion.

Most important, in reading through it I see that we are discussing something that should not be relevant to metalsmith-layouts, I missed that initially. Layouts shouldn't process templating syntax in files, it just wraps files in a template. Metalsmith-in-place processess templating syntax in files in src so if the article overview is in src, it should be processed by in-place. So putting in-place in front of layouts would be the way to go.

By the way, I'm not sure if you're making code changes at the moment. But if you are, please don't merge anything until I or Andrew have reviewed & approved it. That would allow us to share what we know about the ecosystem with you. Otherwise we could be operating under different assumptions and it wouldn't be good for that to affect the code.

webketje commented 3 years ago

@ismay Fair enough, then we agree to disagree on this issue and I will not insist. I also see how such functionality could be handled by a plugin metalsmith-raw-contents. I want to reassure you that I will honor my original commitment , cf. https://github.com/segmentio/metalsmith/issues/335#issuecomment-653654737

For all open PR's/issues I would first pull locally, test, and either reject, validate, or improve the PR in a feature/bugfix branch & ask for approval by at least one of you. I'll be happy to hear additional rules to adhere to