lumeland / lume

🔥 Static site generator for Deno 🦕
https://lume.land
MIT License
1.75k stars 74 forks source link

Filter Pages plugin removes asset files (arguably unexpected -- documentation issue?) #588

Closed hpfast closed 3 months ago

hpfast commented 3 months ago

Version

2.1.2

Platform

linux

What steps will reproduce the bug?

I use the Filter pages plugin to filter out 'draft' blog posts:

site.use(filterPages({
  fn: (page) => {
    return page.data.draft === false && page.data.publish === true;
  }
}));

I am also using the Lightning CSS plugin which, per the docs, loads *.css files as 'assets'.

How often does it reproduce? Is there a required condition?

always.

What is the expected behavior?

Intuitively, since the distinction between 'pages' and 'assets' seems fairly fundamental to Lume's architecture, I didn't expect it to affect 'assets'. I.e. the use of 'pages' in the plugin name suggests it won't affect assets since there is the distinction between site.loadPages() and site.loadAssets().

What do you see instead?

However, the plugin apparently affects all loaded files, and since the CSS files have no metadata, they fail the test and are removed.

Additional information

Of course, this behaviour is perfectly reasonable, and it is easy to work around by checking for asset === false in the filter function. However, It might be a good idea to document this with an example. And perhaps consider changing this in a future major version: rename filterPages to filterFiles, or have two functions filterPages and filterAssets (though filtering assets seems an unlikely use-case)?

If the resolution is just a matter of documentation, I would be happy to provide a pull request if that is helpful for you.

oscarotero commented 3 months ago

Hi. Yes, probably it's unclear documentation. A pull request from you would be awesome, thanks!

Just clarify that the plugin has the extensions option, that by default is *. Maybe the default value should be [".html"] to only filter html pages.

And note that draft pages are ignored automatically, you don't need this plugin for that, unless something is not working fine...

hpfast commented 3 months ago

great, sometime in the coming days I will submit a pull request.

And note that draft pages are ignored automatically, you don't need this plugin for that,

I was porting a rather complicated filter flow from a different project ('publish' is actually a nested data structure with various fields to check), but I forgot that lume checks for draft. This might simplify things a bit, thanks!