11ty / eleventy-plugin-rss

A pack of Eleventy plugins for generating an RSS feed.
https://www.11ty.dev/docs/plugins/rss/
MIT License
92 stars 22 forks source link

Plugin requires "head" filter for rendering templates #50

Closed benjifs closed 2 weeks ago

benjifs commented 3 weeks ago

I was trying out the updates to 11ty@3.0 and this plugin @2.0 and ran into the following issue. On a fairly simple setup, trying to use feedPlugin runs into the following issue for rss, atom, and json.

[11ty] Problem writing Eleventy templates: (more in DEBUG output)
[11ty] 1. Having trouble rendering njk template ./src/eleventy-plugin-feed-eleventy-sample-atom.njk (via TemplateContentRenderError)
[11ty] 2. (./src/eleventy-plugin-feed-eleventy-sample-atom.njk)
[11ty]   Error: filter not found: head (via Template render error)
...

I created a test repo with this setup. https://github.com/benjifs/11ty-3.0-playground

The repo as its setup wont build successfully. If you comment out the following lines to add the missing head filter, it will work.

I would submit a PR for this but not sure if the head filter belongs in this plugin or in the 11ty core.

jeanremy commented 2 weeks ago

Hey, Same problem here. headdoes not seem to be included in the 11ty core, and appears to be used only in this plugin. Maybe just add the filter in the plugin ?

// @11ty/eleventy-plugin-rss/src/virtualTemplate.js
let templateData = {
    ...options?.templateData || {},
    permalink: options.outputPath,
    eleventyExcludeFromCollections: [ options.collection.name ],
    eleventyImport: {
      collections: [ options.collection.name ],
    },
    layout: false,
    metadata: options.metadata,
    // Remove this part ?
    /*
    head: function(array, n) {
      if(!n || n === 0) {
        return array;
      }
      if(n < 0) {
        throw new Error("`collection.limit` option must be a positive number.");
      }
      return array.slice(0, n);
    },*/
  };

  eleventyConfig.addFilter("head", function(array, n) {
    if(!n || n === 0) {
      return array;
    }
    if(n < 0) {
      throw new Error("`collection.limit` option must be a positive number.");
    }
    return array.slice(0, n);
  });
  eleventyConfig.addTemplate.......

Not the best way to make it work I suppose. There must be a reason for the head function presence in the templateData object, but I didn't manage to find it.

zachleat commented 2 weeks ago

Happy to entertain a PR, sounds like we need to rename the head filter used in the virtual templates here: https://github.com/11ty/eleventy-plugin-rss/blob/e1c9a3daff0f4ffe0f9e1054f2038427d4ac3606/src/virtualTemplate.js#L146-L154

This repo is the one!

zachleat commented 2 weeks ago

It sounds as though an application-level head filter is conflicting with the template data head function passed in?

jeanremy commented 2 weeks ago

@zachleat Thanks for your input ! I have tried to rename the head function to something else, but it doesn't work. Not sure to get this right, but to me, it seems to me that the head function in the templateData object cannot be used as a nunjucks filter. I have searched in the plugin and the core repos to find any ways to add filters, and the only way I found was using the addFilter function. I've made a PR, but I would be happy to make changes if you think there is a better solution !

benjifs commented 2 weeks ago

I took some time to try to dig into how virtual templates work but I got lost in trying to figure out how the head filter would apply. It seems to me that virtual templates don't actually create the filter but still can handle and call the function defined in the template data. Based on that, I came up with the following:

The head function defined in templateData can be used if you wrap the reversed collection as the first parameter. So in the following case: https://github.com/11ty/eleventy-plugin-rss/blob/e1c9a3daff0f4ffe0f9e1054f2038427d4ac3606/src/virtualTemplate.js#L18C1-L18C92

You would replace it with:

{%- for post in head(collections.${collection.name} | reverse, ${collection.limit}) %}

Another way of doing it would be to skip the function altogether and call the JS slice function directly like this:

{%- for post in (collections['${collection.name}'] | reverse).slice(0, ${collection.limit}) %}

But that gets rid of checking that limit is a valid number. Mostly just pointing that out because I just learned you can call the JS function directly.

This might look a little messy so creating the actual filter as @jeanremy did in #51 might be better.

I created #53 so we can compare and continue discussing if necessary.

zachleat commented 2 weeks ago

We’ll roll with #51 which seems a bit cleaner. I did rename to eleventyFeedHead to avoid wider conflicts.

Shipping with RSS Plugin v2.0.1

Thank you both for your help!