Refactored to build dynamically (instead of storing file in `./static`)

onecrayon commented 5 years ago

Hey there!

I liked the idea behind your plug-in, but wanted to convert it to use api.afterBuild() similar to the first-party @gridsome/plugin-sitemap instead of storing an RSS file in ./static. While I was doing that, I thought to myself, "Hey, maybe it would be nice to have some actual defaults for the options!", one thing led to another, and well...I might have ended up going a little overboard.

Anyway, this PR makes the following changes:

BREAKING CHANGE: siteUrl must be defined in gridsome.config.js, or plug-in will exit with an error
BREAKING CHANGE: custom options.output.dir input is no longer supported and will result in plug-in exiting with an error
NEW: options.outputPath to specify where the RSS file will live in the built site (defaults to /rss.xml)
NEW: Default function provided for options.feedItemOptions, using standard NodeType attributes and automatically generates the URL based on node.path. Additionally, it will automatically convert relative paths within HTML content into absolute URLs including site scheme and host.
CHANGED: options.feedItemOptions function now accepts a second siteUrl parameter
CHANGED: options.contentTypeName is now explicitly required, and plug-in will exit with an error if it is missing
CHANGED: Build RSS during api.afterBuild(), similar to the first-party @gridsome/plugin-sitemap
CHANGED: Required properties for options.feedOptions are automatically gathered from existing data if they are not present (defaults to siteName and siteUrl from gridsome.config.js)
CHANGED: Basic defaults provided for all required options
CHANGED: Uses fs-extra instead of default Node fs module for writing RSS file

Let me know what you think! I realize this is basically a massive rewrite out of the blue, so if you aren't comfortable with some of the changes I'd be happy to discuss them (or if you're really attached to generating the file statically, I could make an alternative plug-in). :)

darthmeme commented 5 years ago

Hey @onecrayon!

Sorry for the late reply. Thanks for taking time to work on the plugin! Quick question: Do we really need fs-extra? It's an extra dependency and doesn't seem to be doing much more than I was. Or am I missing something?

onecrayon commented 5 years ago

Hey @darthmeme,

Do we really need fs-extra?

Probably not; I mainly went with this because that's how the core Gridsome packages were doing it (e.g. @gridsome/plugin-sitemap). I don't know specifically why they chose fs-extra over fs when it only appears to save about a line of code at the cost of a dependency, though. ¯\_(ツ)_/¯

darthmeme commented 5 years ago

Hey @onecrayon - thanks again for contributing. Going over this again, I don't really think there's a need to introduce breaking changes to the API, so I'm going to go ahead and close this PR.

onecrayon commented 5 years ago

I actually ended up writing my own feed plugin because this one doesn’t support multiple content types or limiting the feed to X most recent items: https://github.com/onecrayon/gridsome-plugin-feed

darthmeme / gridsome-plugin-rss

Refactored to build dynamically (instead of storing file in `./static`) #4